Prevent optional objective to be triggered for opponent when failed

sookmax commented 1 month ago

I've essentially added a check if the player returned by pickWinningPlayer() is the same as the current player and another check if that player is specified in condition.players using matchesPlayerList() in Objective.tsx.

I'm honestly not entirely sure if these checks would cover all the edge cases, but at least the relevant tests are producing desired outputs. I'm also not sure if the checks can be done in checkWinCondition as suggested in https://github.com/nkzw-tech/athena-crisis/issues/35#issuecomment-2143792713.

Sorry, full of uncertainties here 😅

Apart from that, I removed what seems to be a duplicate condition in WinConditions introduced in https://github.com/nkzw-tech/athena-crisis/commit/5771fcb81ccebfc164c25e2282da251d4817cac0#diff-8dde25f248cc37b8a8d1a4562ac42f34b40275047c270622a85dd3e8efcc3f40R770-R774

cpojer commented 1 month ago

Thank you for the PR, and for raising this issue. Your solution makes sense, but it would be problematic in situations where one player achieves an optional objective while also ending the game at the same time. Instead, we should simply ignore checks for objectives in situations where they are optional and verified against the other play.

I went ahead and implemented the correct fix in https://github.com/nkzw-tech/athena-crisis/commit/07f6651f8c70f83a37f99216ebf0ed047516431c

sookmax commented 1 month ago

Thanks for correcting my PR! I just took a look at https://github.com/nkzw-tech/athena-crisis/commit/07f6651f8c70f83a37f99216ebf0ed047516431c, and I realize the important thing I was missing was the fact that the counter win can only happen when the action is destructive. Never occurred to me to apply ignoreIfOptional selectively to those escort and capture missions only when the action is destructive! I learned a lot, thanks 😁

nkzw-tech / athena-crisis

Prevent optional objective to be triggered for opponent when failed #44