-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CR] Hordes don't wander to 0,0 #74053
[CR] Hordes don't wander to 0,0 #74053
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
If serialization depends on all groups having the same target the serialization logic is broken and needs to be fixed. Hordes shouldn't be synchronized even if they're enabled. Sneaking hordes back in for people who disabled them and then later on enable the hordes anyway would be a dastardly deed. |
I'm not sure I conveyed it properly. This won't enable wandering behavior for worlds with it disabled, wandering behavior is technically always on. This just tells them to wander to their own location instead of to 0,0. The serialization issue can be summarized by simply posting an example. Here's without the patch:
And here's with the patch:
We save a lot of save data (no pun intended) by grouping the hordes together when their type, population, and horde values are the same, then serializing them as one huge block of tripoints. When it is deserialized instead of reading each group individually, it simply goes through the tripoint list and copy-pastes the same group into each spot, since it knows each group (of this particular binning) is the same. When they have different targets it cannot do that. The code in question may be more informative than my ability to explain: Cataclysm-DDA/src/savegame.cpp Lines 368 to 378 in 388ca6c
|
Your description of the movement enabling wasn't particularly clear (to me at least). If, however, it means they wander in place, that should be fine. If you're only binning matching hordes that would be reasonable. I'd address the differing destinations by replacing the current absolute tripoint locations with a pair of location and destination, allowing you to still factor out the common part. |
Might also fix #20363? At least the first part. I think it's two separate issues meshed together and not explained well. |
Why do they need to wander anywhere? Can't they just transform to horde once they leave the reality bubble, instead of menacingly moving to that direction they picked, sometimes ignoring everything else like enemy (character/npc) presence? |
Wandering behavior is very very tied into our systems. For example, when you make noise that they can't see - wandering. If they're chasing you but you duck out of sight - wandering. Wandering is what causes them to do the moving. They're set to wander towards 0,0 when they transform from being a [horde] into actual [monsters]. What this PR has done is instead of going towards 0,0... they simply wander towards their current position. This is also not accomplishable by setting wander desire - because of course it's possible to modify wander desire without modifying the wander destination. And if that happened... they'd just start sprinting towards 0,0 again. |
Summary
Bugfixes "Hordes won't wander without a reason"
Purpose of change
Hordes(mongroup) would most often be initialized without specifying a target. This meant that the default value would be used - 0,0.
Cataclysm-DDA/src/mongroup.h
Lines 135 to 140 in de4ec39
With wandering hordes set to on, hordes would receive a new target every 2.5 minutes. However, if wandering hordes was set to off... that never happened. (Most*) every horde would be laser-targeted as tripoint_abs_sm 0,0. Thus, depending on your orientation from 0,0 they would run in a specific direction.... this is why the reports were so confused.
Describe the solution
Don't rely on default constructed value of 0,0 to set the target of the horde.
Don't rely on wandering hordes being set to on, either...
Construct the default target of the horde as its position if target is unspecified.
For existing savegames, check if their targets are 0,0 and set target to the current position if so.
Describe alternatives you've considered
Get rid of the wandering hordes option, force it always on.
Testing
Load the provided save from #73725, check nearby hordes with debug. Their targets are all their current position instead of 0,0.
Additional context
Our monster group serialization (overmap::load_monster_groups specifically) will probably not like this very much? As each group will have a separate target, instead of serializing them all into one object they'll each need a separate object as they have different targets.
The debug menu additions were actually not at all helpful here, but no reason to throw them out.