Help wanted: Source Keepers overhaul
-
@artch It seems to me like you will have to pick 2 out of these 3:
- keep SKs exposed to melee attacks at all times
- make SKs' movement as simple as possible
- keep lairs where they are
-
@eduter Exactly. Very interested in community opinion on what's more important.
-
@artch Mind sharing what exactly you think the issue with moving the Pathfinding to the processor is? I'm still fiddling with the server code to understand how exactly NPCs are integrated into the flow of the game, but calling the pathfinder differently seems like a fairly small issue to me, so maybe I'm missing something here.
Also what exactly is the overhead here? Is it the creation of the
vmfor each SourceKeeper or the setup of the API wrapper usercode has? Is it an option to just remove thevmlayer for NPCs?
-
@postcrafter Runtime overhead is created not only by
vmmodule itself, but by many other factors: working with inter-cluster user execution queues, fetching database queries, managing separate runtime processes with their own timers, etc.Adding full path finding (i.e. feeding full room grid to real A*) to processors is inappropriate since processors are meant to be very fast, and a single heavy path finding call could be longer than an entire room processing cycle. Also,
@screeps/pathfindinglibrary has some overhead like initializing heaps, and doing that in processors is no good. Better to make something simple instead.
-
I think it should be possible to keep the SK movement very simple (local greedy search). That means an SK might get stuck in a loop and never make it to the Source or Mineral, but it should be rare so it won't ruin the fun. It might even enhance the fun with crazy techniques to send SKs on wild paths.
This would keep the SK vulnerable to attack (same movement restrictions as any other creep they can't move onto walls, etc). The movement code is dead simple
move in dir of src, if blocked by wall, move diagonally to src, if diagonal blocked by wall then move random. The same brain dead code can be used to pursue player creeps if we want to allow that. SK lairs don't need to change and could be consolidated in the future with no changes.All this assumes that the processor does some intent validation. E.g. I believe it is the processor that decides which creep win when two creeps try to move onto the same square during a tick.
I think targeting creeps might be a harder problem. I believe the LOOK and FIND indexes won't be available in the processor. How will the creep discover which creeps are in the room with it? Without the LOOK indexes it might be very expensive to loop over all creeps in the room checking for range.
-
@deft-code Making LOOK and FIND indexes requires looping through objects as well, so there's no difference, either NPC processor or runner should do that anyway.
We're now debating whether it's viable to leave their movement by plain land as usual, but without dynamic path finding. I.e. the path is calculated once and then reused, and if the keeper is blocked, it will stuck until it kills the blocking creep or die.
-
@eduter said in Help wanted: Source Keepers overhaul:
@artch It seems to me like you will have to pick 2 out of these 3:
- keep SKs exposed to melee attacks at all times
- make SKs' movement as simple as possible
- keep lairs where they are
I would take the first two options and move some lairs:
- keep SKs exposed to melee attacks at all times
- make SKs' movement as simple as possible
A vast majority of players will not even notice if some lairs are relocated.
-
OK, the task specification has been changed. Let's settle on this:
leave their movement by plain land as usual, but without dynamic path finding. I.e. the path is calculated once and then reused, and if the keeper is blocked, it will stuck until it kills the blocking creep or die.
The first post is updated.
-
@artch said in Help wanted: Source Keepers overhaul:
leave their movement by plain land as usual, but without dynamic path finding. I.e. the path is calculated once and then reused, and if the keeper is blocked, it will stuck until it kills the blocking creep or die
This makes the logic a lot easier to implement. I also thinks this looks a lot like the current implementation.
-
@artch said in Help wanted: Source Keepers overhaul:
OK, the task specification has been changed. Let's settle on this:
leave their movement by plain land as usual, but without dynamic path finding. I.e. the path is calculated once and then reused, and if the keeper is blocked, it will stuck until it kills the blocking creep or die.
The first post is updated.
Is there any test data provided for this task or do we have to generate our own?
Or is there any information about how sources and source keeper lairs are generated in SK rooms?
-
I've started working on this, progress can be viewed in the pull request on the engine and driver. I plan on cleaning up some of the code and cache the paths properly before marking it as ready to merge in.
The source keepers should already act like they would normally do, but I can't confirm as I lack source keeper harvesting code, therefore I appreciate any feedback on this.
If anyone wants to test the changes, feel free to check out the
source-keeper-to-processorbranch on my screeps-server-dev repo which will help with setting up a server quickly.
-
Dang, I didn't realize that your PRs had been out for a month, @PostCrafter. @artch any idea of when this will be tested/merged?
-
@semperrabbit It's not optimized in any ways yet (paths aren't getting saved to database yet) and there are a few other things I would change before merging it in. Due to my experience with past pull requests I don't feel like spending more time on this until I get some sort of official response whether my changes are going into the anticipated direction at all.
-
I'm sorry we can't afford spending time reviewing unfinished work, community contributions are supposed to save resources, not consume them. The direction seems to be fine (the only thing which concerns me is modifying
processor.js), but the devil is in the details, and I can't dive into the details right now, too many things require attention.
-
Do the source keepers still need an overhaul? Are they still a large portion of server load?
I sketched out the code and it's fairly straight forward, but I don't want to go through the hassle of a pull request if interest has waned or they're no longer a problem.
-
The code is still needed @deft-code
the reward is still up for grabs.
-
I've got a clone about ready for a pull request https://github.com/deft-code/engine/tree/keepers. The logic is all done, I just need to test it out, and get someone to review it.
Two things: First. The SourceKeeper:* users are configured outside of the engine. I'm planning to just ignore those users but continue using userID 3. This should be fine, but if the SourceKeeper users remain configured they'll still incur some runtime overhead, (Those users will throw an exception every tick when
require('main')is called by the engine). I could instead actively skip the SourceKeeper users, but it would cleaner to just assume they no longer exist. If the extra users will be an ongoing problem It might be worth it to add a check that skips running user code if there is not main module defined.Second: The current SourceKeeper and Invader code don't use a loop function. I suspect that might be a big part of their overhead. A less invasive fix would be to update their code to use a loop and possibly make all the SourceKeeper:* users share a single global (to avoid recompiling their code for each room).
If I were directing the project I'd try out the second option first, unless the plans are to move all the bot code into the processor. There is kind of a purity to coding the bots using the same API given to players and it would be nice to make that work better rather than bypass it.
-
Pull request is live. https://github.com/screeps/engine/pull/96
I'm still interested to know if using a main.loop with the keepers or invaders lowers their cpu burden. I talked with @ags131 who runs screepsplus (the largest private server) and he doesn't currently have the ability to monitor SK cpu burden or run a forked engine. It looks like the dev team might be only one who can verify this without some investing a lot of time.
If there is interest I can sent another PR that just updates the SK code to use a main.loop (very low risk), though honestly it'd probably just be easier for the devs if they made that small edit themselves.
-
Very interesting
-
Strongholds question: but very SK rewrite specific.
Is the stronghold code running in a VM or in the processor?\
I'm assuming in a VM.
Now that there is the concept of per sector bot. It might make sense to refactor the invader and SK code to run once per sector instead of once per room. I think that will lessen the burden on the servers (It seems like there is significant CPU and Memory overhead for running a use vm).
My SK rewrite didn't really have that much more efficient logic but it does bypass the vm overhead (compiled code, Memory, intent queueing). Amortizing the vm overhead across an entire sector might be enough to fix the server burden.
It would also let me give up on testing the SK logic. I'd much rather have bots using the player api than than getting my PR accepted.
On a side note I hate manual testing, I'd gladly split the sub token bounty(3 subs) with anyone willing to test the PR for me. Assuming of course the devs still want in processor SK logic.
