Skip to content

Conversation

@Andrene
Copy link
Contributor

@Andrene Andrene commented Aug 7, 2022

Should fix issue #176

@cclauss
Copy link
Collaborator

cclauss commented Aug 7, 2022

Why not UnitDiedEvent.killing_player_id as mentioned in CHANGELOG.rst?

@Andrene
Copy link
Contributor Author

Andrene commented Aug 7, 2022

The changes I made were to handleUnitPositionsEvent and handleUnitDoneEvent, neither of the associated events processed by those have either attribute. In the above commit I fixed the reference here

event.killer_pid, Length(seconds=event.second), event.frame
I dont think that killing_player_id or killer_pid would be a good fit there, as the error that is logged references being unable to delete a unit index, and the if condition above checks if the unit_id_index is in the list of active units, so to me it'd make sense to use the unit_id_index

Further down in the handleUnitDiedEvent function an error for unknown killing player id exists and uses the appropriate non-deprecated attribute

event.killing_player_id, Length(seconds=event.second), event.frame

Copy link
Collaborator

@StoicLoofah StoicLoofah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@StoicLoofah StoicLoofah merged commit 7da58b1 into ggtracker:upstream Aug 20, 2022
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.

3 participants