Fix event random numbers never stored in setRandomNumbers() - #1217
Open
acato wants to merge 1 commit into
Open
Conversation
EventTriggeredData::setRandomNumbers() creates a RandomContainer for each event in the trigger, generates a random number via getSorenRandNum, but never actually stores the container in m_RandomNumbers. The push_back call was missing, so m_RandomNumbers remained empty after the function completed. This caused getRandomNumber() and getRandomNumberForIndex() to always return 0 (the fallback default), which had two consequences: 1. Any Python PythonCanDo callback using getRandomNumberForIndex() for probability checks would always see 0. For example, canTriggerVolcanoDormant1() checks "getRandomNumberForIndex(0) < 250" which was always true (0 < 250), making the volcano dormant event fire 100% of the time instead of the intended ~25%. 2. The DLL-side TriggerChance fastpath in CvPlayer::canDoEvent() (line 14762) uses getRandomNumber(eEvent) to check event probability thresholds. With the vector always empty, this also always returned 0, bypassing intended probability gates for events validated through that code path. The fix adds the missing m_RandomNumbers.push_back(container) so that generated random numbers are actually persisted in the vector and available to both Python and C++ callers. Related: We-the-People-civ4col-mod#1205 (CvRandomInterfaceEvent tuple index out of range) The primary cause of We-the-People-civ4col-mod#1205 was an argsList index mismatch fixed in 38b7518, but this bug compounded the issue by making the random number check in canTriggerVolcanoDormant1 a no-op. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
@Nightinggale could you have a look at this ? |
Collaborator
|
I assume we should do this fix. It could have influence on the existing events, but I have checked if there are any events which use probabilities via
but I could not find any or only the volcano event and very few that still use TriggerChance. Hence the effect should be limited. We need to monitor if any events have unintended issues after the fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EventTriggeredData::setRandomNumbers()creates aRandomContainerfor each event, generates a random number, but never callsm_RandomNumbers.push_back(container)— the vector stays emptygetRandomNumber()andgetRandomNumberForIndex()to always return 0 (the fallback default)canTriggerVolcanoDormant1fires 100% instead of ~25%)Details
The bug affects two code paths:
getRandomNumberForIndex()— e.g., the volcano dormant event always triggers because0 < 250is always trueTriggerChancefastpath inCvPlayer::canDoEvent()which usesgetRandomNumber(eEvent)— always returns 0, bypassing probability gatesThe fix is a single missing
push_backcall.Test plan
TriggerChancefire at their intended probability rather than always/neverRelated: #1205
🤖 Generated with Claude Code