dfhack
dfhack copied to clipboard
DFHack
[WIP] Refactors EventManager to support sending on multiple event frequencies
This adds code for saving event data for future ticks, and the updated callbacks sending the appropriate saved data.
edited: undo of formatting and force pushed, description changed accordingly
didn't seem like something to open a PR over but I've been wrong before..
I can see already that it would have been better in separate PRs. Always separate refactoring/reformatting PRs from business logic changes. Otherwise the review feedback gets muddled.
it would have been better in separate PRs
Alright, I resorted things back to how they were.
I'm still a little wary of this change, especially with how it changes the guarantees of consistency (i.e. objects referred to in the events actually still existing). My concern is that any issues with this change won't show up until after a release, and then there could be a fire drill in getting a fixed release out and updating all the starter packs.
I think we need to give ourselves some better mitigation options. Something that players can do themselves to fix things if the new behavior causes issues for them.
Could the behavior be toggled with a command players can run from dfhack.init? We can merge with the flag defaulting to false, then ask people who use mods that use EventManager to test. Then we can default the flag to true, then after a few releases with no reported issues, we can remove the old behavior entirely. How does that sound?
Sounds like a simple way to avoid problems. Maybe I could additionally spend a bit of time and make comments about known life cycles in the managers.
Ok so I didn't put the comments in the managers, I thought it would make more sense to centralize the information to one location. So I put it with the event_tracker declarations
So at some point in revising towards an ability to throw out stale id references I began refactoring various managers to share scan loops. Might be a week or two.. I guess we'll see. I'm knee deep in the code for all the report related stuff.
I just need to finish transposing some intricately detailed code to these new scan functions, spot as many optimization zones as possible, then simplify the logic and expressions.. it's probably 400 lines of code to move
just need to finish transposing
So that's done. I'll finish off the todos next.
I deal with some stale id's, but I haven't touched most of the event data yet. There is also some code I think I might have some luck simplifying.. and some comments I need to add.
Afterwards I'll try to find an easy way to split this into two PR's. One with the scan centralization, and another with the data structure changes and changes to the managers.
Speaking of the managers, I changed them all to send.*Events. Mostly to keep track, but also I think it makes more sense; not sure whether I should change them back or not.
Something that players can do themselves to fix things if the new behavior causes issues for them.
Could the behavior be toggled with a command players can run from dfhack.init?
There's also this of course, I haven't forgot (yes I did).