Skip to content
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

Merge hotfix into master (add debug info to traitor activation) #33178

Closed
wants to merge 19 commits into from

Conversation

Errant-4
Copy link
Member

@Errant-4 Errant-4 commented Nov 5, 2024

About the PR

Merge hotfix #33119 into master

Adds various debug logging during traitor activation, no gameplay changes

Requirements

Jezithyr and others added 18 commits October 12, 2024 22:21
space-wizards#32793)

* Fix some rounds failing to end due to mind roles

Fixes space-wizards#32791

This is caused by ShowRoundEndScoreboard running into a bug trying to display antags: some player is showing up as antag with MindIsAntagonist(), but has no antag roles listed in MindGetAllRoleInfo().

This was caused by one of the roles of the player having the Antag boolean set, but having no AntagPrototype set.

The responsible mind role appeared to be MindRoleSubvertedSilicon which is missing a set for the SubvertedSilicon antag prototype.

I also added resilience to the round-end code to make it so that an exception showing the scoreboard (and sending the Discord message) would not cause the round end logic to completely abort from an exception.

I am planning to add an integration test to cover this bug (no prototype in mind roles), but I'll leave that for not-the-immediate-hotfix.

* At least one maintainer approved this tiny PR without reading it, not naming names.
* fix ninja bomb component check

* remove TryGetRole
…2882)

Fix: Plushies no longer delete items when recycled (space-wizards#32838)

fix

Co-authored-by: beck-thompson <[email protected]>
Update submodule

This fixes an important memory leak.
…tem. (space-wizards#32942)

Fix loneop spawnrate by reverting it to not use the custom shuttle event system.
* Add debug messages to traitor activation

* more debug
@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Nov 5, 2024
@Errant-4
Copy link
Member Author

Errant-4 commented Nov 5, 2024

Of note, there is a single, unplanned line break change to ThiefBeaconSystem. It does not come from the hotfix. While it is sort of irrelevant, it should be investigated what is the source of this discrepancy, to ensure something like that does not happen again in the future, with a potentially more severe impact

@Errant-4 Errant-4 added the S: DO NOT MERGE Status: Open item that should NOT be merged. DNM. Allows test to run unlike draft. label Nov 5, 2024
@VasilisThePikachu
Copy link
Member

VasilisThePikachu commented Nov 5, 2024

Of note, there is a single, unplanned line break change to ThiefBeaconSystem. It does not come from the hotfix. While it is sort of irrelevant, it should be investigated what is the source of this discrepancy, to ensure something like that does not happen again in the future, with a potentially more severe impact

It was from jez improperly cherry picking a pr and its now stuck around. We can only fix it with a force push

@Errant-4
Copy link
Member Author

Errant-4 commented Nov 5, 2024

Of note, there is a single, unplanned line break change to ThiefBeaconSystem. It does not come from the hotfix. While it is sort of irrelevant, it should be investigated what is the source of this discrepancy, to ensure something like that does not happen again in the future, with a potentially more severe impact

It was from jez improperly cherry picking a pr and its now stuck around. We can only fix it with a force push

Okay that sounds fine, then. Should we add an extra commit here to add that deleted line back, so this does not cause a new difference between stable and master?

@Errant-4
Copy link
Member Author

Errant-4 commented Nov 5, 2024

There, perfectly lined up now.

@Errant-4 Errant-4 added S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. and removed S: DO NOT MERGE Status: Open item that should NOT be merged. DNM. Allows test to run unlike draft. labels Nov 5, 2024
@Errant-4
Copy link
Member Author

Errant-4 commented Nov 8, 2024

This is no longer needed because of #33218

@Errant-4 Errant-4 closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants