search

tewtal / SMZ3Randomizer

Super Metroid & A Link to the Past Crossover Item Randomizer V11
https://samus.link/
MIT License
75 stars 32 forks source link

Move World setup out of Filler #185

Closed kedNalatacId closed 2 years ago

kedNalatacId commented 2 years ago

Sorry for last pull request; forgot to update local branch. Rest of the text is the same. The "doesn't merge cleanly" should have been a clue.

Feel free to ignore this request, just a suggestion.

It became clear as part of the new reward code that world setup is inside of the Filler object instantiation. This is a side-effect which means it's not obvious. Moving world setup into Randomizer.cs seems cleaner/more obvious.

While here, doing a little bit of cleanup on the world creation loop (bifurcate on playername, rather than the entire loop, since the loop has to run at least once anyway).

RebelusQuo commented 2 years ago

Both #178 and #183 are relevant to this. I'm waiting for @tewtal to resolve.

RebelusQuo commented 2 years ago

Was this part of some bigger change you have in mind? It seems trivial on its own.

kedNalatacId commented 2 years ago

No larger change; just stuck out to me as a little odd.

RebelusQuo commented 2 years ago

In that case I don't see that motivation. The Filler distributes items in such that there is at least one valid route. This also has to include the prize and medallion access arrangement since this route is directly affected. Thus it makes perfect sense for the Filler to "setup the world".

This is in addition to the practical matter, as I mentioned, of my PR (which is more substantial) already conflicting with tewtal's changes on master. In short, thank you for the suggestion but I will close for now.