Fix hint generation bugs on develop-rando #4001
Conversation
1. Fixed Ganon Non-hint text from loading as Saria's Magic Hint. 2. Fixed Ganon Non-hint text from not getting saved correctly. 3. Fixed gossip stone hint generation from not generating any non-always hints on No Logic. For #3, the hint distribution and placement algorithm was bailing out too early when it wasn't able to place a hint. For No Logic, what it was doing was failing to place WOTH hints (since No Logic seeds don't calculate WOTH candidacy), returning the amount of hints it failed to place, and then it called the function to redistribute the hints, but did not call the function to attempt to place the remaining hints. Additionally, it was not accounting for the fact that we shouldn't redistribute the hints into the categories we failed to place a hint in, so it would redistribute hints right back into those categories. I changed it so that when DistributeHints gets called after PlaceHints fails to place the hint, it checks if the distribution settings copies attribute was set to 0. In this case, it breaks while looping for the type distribution settings, and moves on to the next category. Also, it now repeatedly attempts to distribute and place hints until PlaceHints returns 0 (meaning it placed all of its hints successfully).
| @@ -963,7 +964,7 @@ static void DistributeHints(std::vector<uint8_t>& selected, size_t stoneCount, s | |||
| for (uint8_t distribution = 0; distribution < distTable.size(); distribution++){ | |||
| currentWeight -= distTable[distribution].weight; | |||
| if (currentWeight <= 0){ | |||
| if (stoneCount >= distTable[distribution].copies){ | |||
| if (stoneCount >= distTable[distribution].copies || distTable[distribution].copies == 0){ | |||
There was a problem hiding this comment.
I assume this is to catch an edge case, but i can't think of any case where this would trigger as it implies that stoneCount is in negative numbers which is an erroneous state in it's own right.
Even then, if copies is zero and the hint type has weight that's also an erroneous state, albeit one that should be accounted for, specifically a hint distribution is declared but will never be assigned stones, so there is no reason to select it instead of letting the stone check fail and the code remove it from consideration.
There was a problem hiding this comment.
When it fails to place a hint in PlaceHints it sets copies to 0 for that distTable entry. This was the only way I had to make it not distribute hints somewhere that it failed to place a hint in the previous cycle.
There was a problem hiding this comment.
Without this check, it was re-distributing hints back into a distribution that it failed to place hints in earlier.
| @@ -1074,8 +1075,9 @@ void CreateStoneHints() { | |||
|
|
|||
| while(totalStones != 0){ | |||
There was a problem hiding this comment.
Doesn't the change directly below this line make this redundant now? Cleaning up this second to have distribute first in the loop would better be a larger change, something like this, replacing lines 1074 through 1082.
| while(totalStones != 0){ | |
| while(totalStones != 0){ | |
| DistributeHints(selectedHints, totalStones, distTable, hintSetting.junkWeight); | |
| totalStones = PlaceHints(selectedHints, distTable); | |
| } |
There was a problem hiding this comment.
I think the main issue is that we have to attempt one cycle in order for totalStones to have a value to check for the whiel loop. That being said this does sound like the perfect case for a do-while loop.
There was a problem hiding this comment.
Oh nevermind, forgot about the initial totalStones value coming from GetEmptyGossipStones.
There was a problem hiding this comment.
There is one thing that's different about the DistributeHints call inside vs outside the loop, and that's the extra false at the end. That false prevents re-adding any fixed hints that were added in the first call to DistributeHints.
For point 4, the hint distribution and placement algorithm was bailing out too early when it wasn't able
to place a hint. For No Logic, what it was doing was failing to place WOTH hints (since No Logic seeds
don't calculate WOTH candidacy), returning the amount of hints it failed to place, and then it called
the function to redistribute the hints, but did not call the function to attempt to place the remaining hints.
Additionally, it was not accounting for the fact that we shouldn't redistribute the hints into the categories we failed to
place a hint in, so it would redistribute hints right back into those categories. I changed it so that when DistributeHints
gets called after PlaceHints fails to place the hint, it checks if the distribution settings copies attribute was set to 0.
In this case, it breaks while looping for the type distribution settings, and moves on to the next category. Also, it now repeatedly
attempts to distribute and place hints until PlaceHints returns 0 (meaning it placed all of its hints successfully).
Build Artifacts