Skip to content

Conversation

@cppcooper
Copy link
Contributor

Fixes #2031 by way of git checkout master -- library/modules/EventManager.cpp then re-adding the new event types.

@myk002
Copy link
Member

myk002 commented Mar 29, 2022

does this keep the new behavior (freq per handler) for tick events? or is that too complicated to add back without changing the other events too?

@cppcooper
Copy link
Contributor Author

cppcooper commented Mar 29, 2022

This keeps the old behaviour. All I did was literally checkout the file from master and add the additional event types. No refactoring was done (aside from renaming the NEW_UNIT_ACTIVE => UNIT_NEW_ACTIVE so that the naming scheme was consistent with the existing UNIT_DEATH)

edit:
Though I think there is an unused when member now.. since I didn't change the header back

@myk002
Copy link
Member

myk002 commented Mar 29, 2022

@cppcooper
Copy link
Contributor Author

Tick Events use a special queue (unordered_multimap iirc). They calculate when they need to fire and then enqueue with that as the key. I might be getting details wrong but I'm 99% sure that the tick events can be any freq and it'll be obeyed.

The other events in that plugin don't use non-zero frequencies.

#1918 relied on #1876 because of the event types it uses. It needs to look at jobs when they are assigned a worker, not just when they are created. I can see how confusion would arise, I play it a bit loose with my PR contents

@myk002
Copy link
Member

myk002 commented Mar 29, 2022

ok, cool. understood.

after I merge this, #2044 will be very unhappy. you might want to copy the changed files there somewhere safe, merge current develop (after I merge this PR) and not care about conflicts. Then you can copy your saved files back.

@myk002 myk002 merged commit b18eff9 into DFHack:develop Mar 29, 2022
@myk002 myk002 mentioned this pull request Mar 29, 2022
@cppcooper cppcooper deleted the fix-2031-revert branch April 19, 2022 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EventManager can miss sending events to handlers

2 participants