Archipelago
Archipelago copied to clipboard
Aquaria: mega refactoring
What is this fixing or adding?
This PR is mainly refactoring. Here is what changed:
- Changing item names so that each words are capitalized (
Energy Forminstead ofEnergy form) - Removing duplication of string literal by using:
- Constants for items and locations,
- Region's name attribute for entrances,
- Clarify some documentations,
- Adding some region to be more representative of the game and to remove listing of locations in the rules (prioritize entrance rules over individual location rules).
This is the other minor modifications that are not refactoring:
- Adding an early bind song option since that can be used to exit starting area.
- Changing Sun God to Lumerean God to be coherent with the other gods.
- Changing Home Water to Home Waters and Open Water to Open Waters to be coherent with the game.
- Removing a rules to have an attack to go in Mithalas Cathedral since you can to get some checks in it without an attack.
- Adding some options to slot data to be used with Poptracker.
- Fixing a little but still potentially logic breaking bug.
How was this tested?
- By running unit testing
- By doing a lots of generations with different options
- By playing multiple runs, including solo and multiworlds
You may want to update the early option docstrings, since the parenthetical doesn't exactly apply
You may want to update the early option docstrings, since the parenthetical doesn't exactly apply
True. Thank you.
just did a simple code review, no fine tooth combs on any of the changes nor any testing/generating
changes looked reasonable and no obvious issues (barring two nitpicks below) only questions i have are:
* why not add region name enums too? seemed a little weird to go through all the item/location name changes, and then see string literals for the regions * adjusting urns/crates rules are removed and I didn't see where they were moved to, did i miss where that was or were these not neededelse the nitpicks are nitpicks, I think the option comparison one should be fixed but I'll let core decide to enforce it or not in this PR
The idea was to remove the duplication of literal string. With location and with items, the constants was a good way to do it because there is no named object that individually contain locations and for items, there is item objects, but they are created in a loop, at generation.
For region, this is different because there is named object that I can used to individually address them. So, creating constants as well would uselessly duplication the informations.
Now, for the urns and crates, there is now a region that contain them all and those regions have the requirement to be able to break those urns and crates.
@Exempt-Medic @qwint Just to let everyone know that I add a new commit. In this commit, I add 2 options to the slot data so that it may be used with a Poptracker that is being developped. I also fix a little logic bug.
I have execute 100 generations to tests the new release.
Sorry about all the new commits. We are doing a poptracker for the game and I receive some requirements from the tracker's dev. It should be the last one here. I think.
I have relaunch the tests and generate another 100 generations with randomized options without any problem. Also, the client connect to the server without any problem.