Start splitting Location List into ShipInit functions#5011
Start splitting Location List into ShipInit functions#5011Malkierian merged 7 commits intoHarbourMasters:developfrom
Conversation
soh/soh/OTRGlobals.cpp
Outdated
| SohGui::SetupGuiElements(); | ||
| ShipInit::InitAll(); | ||
|
|
||
| Rando::StaticData::InitHashMaps(); |
There was a problem hiding this comment.
| Rando::StaticData::InitHashMaps(); | |
| Rando::StaticData::InitHashMaps(); |
There was a problem hiding this comment.
??? I'm not understanding why the space? That make it misaligned with the line directly above it?
There was a problem hiding this comment.
Oh weird I looked at it on a different page and now I see it looks misaligned. I'll have a closer look tomorrow.
There was a problem hiding this comment.
Should be fixed now
briaguya0
left a comment
There was a problem hiding this comment.
it's awesome seeing giant files split up! thanks for doing this!
|
Just one question, has this been tested with a number of extra locations that would have exhibited the overflow issue? |
|
Not in this exact permutation, but I have confirmed with Spoon that when crates were crashing on Debug builds, moving them to a separate function did prevent the crash. We can ask Spoon or serprex to rebase onto this and adopt this pattern if we want to be super sure. |
Malkierian
left a comment
There was a problem hiding this comment.
Nah, that's good enough for me. We can fine-tune it if necessary later. Rather get this into develop for them than have them have to rebase back and forth.
InitLocationList was getting so big that the PRs for Grass and Crate shuffle would crash in Debug Mode due to a StackOverflow. Combining the two would cause a crash even in Release mode. Splitting InitLocationList into multiple functions was the Impetus for this PR, and while I was splitting it up anyway I opted to make use of ShipInit for this. New types of checks should use this pattern and eventually we should find a place for the other types of checks to use this pattern (Skulls, Frogs, Chests?, etc.). For now InitLocationList still exists but it only has the older checks and should never be added to. If anything efforts should be made to move the remaining checks out.
Build Artifacts