Added new announcement to fixUnit in fix/loyaltycascade#1575
Added new announcement to fixUnit in fix/loyaltycascade#1575Kyrozis wants to merge 1 commit intoDFHack:masterfrom
Conversation
|
|
||
| if calmed and not fixed then | ||
| dfhack.gui.showAnnouncement( | ||
| ('loyaltycascade: %s is now removed from miscellaneous conflicts'):format(unit_name), COLOR_WHITE) |
There was a problem hiding this comment.
I think this would be better wording:
%s has been removed from one or more existing conflicts. (%s may re-join existing conflicts.)
SilasD
left a comment
There was a problem hiding this comment.
"Although I will admit that the possibility of a Loyalty Cascade scenario is extremely unlikely, I remain uncomfortable with the..."
"[PLAYER_NAME] doesn't need to hear all this. He's a highly trained professional. We have assured the Administrator that nothing will go wrong."
"I never thought I'd see a Loyalty Cascade, let alone create one."
"Why didn't they listen? We tried to warn them".
I think this change is okay in the context of the tool's current capabilities and implementation.
I think adding Announcements instead of printing messages about what it did is too... cutesy, I guess. That was one thing that caused yesterday's confusion about what the tool does and how it works. Discord discussion
Adding Announcements per-unit is fine, but I think the tool should also print a summary to the console. Without any immediately-obvious output, a user can think the tool is broken.
And I lean toward the opinion that quelling all conflicts is outside the remit of the tool. A tool named fix/loyalty-cascade should only (a) fix loyalty cascades, and (b) report on whether or not it even found a loyalty cascade in progress. Especially as the loyalty cascade bug has been fixed, or mostly fixed.
If loyalty cascades no longer occur, the tool should be removed, not extended.
| ## New Tools | ||
|
|
||
| ## New Features | ||
| - `fix/loyaltycascade`: Added announcements for units who aren't renegades but still got removed from conflicts by makeown.remove_from_conflict in fixUnit |
There was a problem hiding this comment.
I think this is too technical. I suggest removing the bit about by makeown.remove_from_conflict in fixUnit.
Added an additional announcement to fix/loyaltycascade for units who didn't turn against your civ but got removed from conflicts nonetheless by makeown.remove_from_conflict within fixUnit