Hades: Implement Game

[NaixGames]

What is this fixing or adding?

Adds Archipelago support for the game Hades.

The mod for the game and install files can be found in: https://github.com/NaixGames/Polycosmos

How was this tested?

Several test sessions by myself and members of the community. This followed by continuous updates to the apworld and client based on feedback and bug reports.

If this makes graphical changes, please attach screenshots.

[ScipioWright]

Some comments to start off. Needs updating to the new options API.

[Zunawe]

Some overall comments rather than going through and marking too many individual things.

• The option descriptions could use a pass for fixing spelling, grammar, and punctuation.
• Some display names are Pascal cased class names instead of human-friendly names. Some option descriptions refer to the Pascal cased names.
• If the mod doesn't rely heavily on item and location names being Pascal cased instead of human-friendly, I have a very strong preference for changing those names to make them more readable and less programmery. If the mod does rely heavily on item and location names being Pascal cased, I would strongly recommend changing the behavior to rely solely on IDs instead and then fixing up the names.
• I might say the same for region/entrance names, mostly because you're just using string literals here and not trying to create them dynamically or match them up with variable names anywhere. So they might as well be easy to read. But this one matters less, since users are much less likely to encounter them.
• There are a bunch of instances of variable names spelled elyseum instead of elysium. The same is true for regions and entrances.
• A linter would have a field day with this code. You should check AP's style guide and find a linter to point out or fix the easy stuff. Some of the more prevalent things I'm seeing are:
  • You should have spaces around binary operators and preferably parentheses where order of operations matters, even if it's correct as-is (e.g. 1073 + i + (subfixCounter * 73) instead of 1073+i+subfixCounter*73).
  • Comments should have a space after the #.
  • Indentation is sometimes 4 spaces and sometimes 8 (should be 4).
  • if statements, while loops, and for loops should by default not have parentheses around their conditions.
  • Some variable names are camel case when they should be snake case.
  • A couple attribute names are all lowercase (e.g. roomweaponbased) instead of snake case.

There are some things with the world that I'll eventually comment on, but even just running a linter will make a more thorough review much more approachable.

[NaixGames]

I just did a pass attempting to address some of @Zunawe comments. I might have missed some formatting ones though. Hopefully it is better than when they saw it the other day.

As a important comment; the apworld used names in Pascal case since Hades used names in pascal cases itself for its items. So I could really easily cast names from Archipelago back and forth with Hades without worrying about converting or parsing them. I changed that by parsing the names for locations before getting send from Hades to AP and when Hades gets the item names it erases all spaces. The change might not be completely understand here (since that change is made only on Hades side) but that has, thankfully, being addressed.

[Zunawe]

Looks much more appealing, thanks.

As a important comment; the apworld used names in Pascal case since Hades used names in pascal cases itself for its items.

I understand. In my non-core opinion, even though we currently have a 1-to-1 mapping of IDs and names, IDs should be the single source of truth for identifying an item/location in a game. The ID has no purpose except to identify, so it should be used as the identifier for as long as you're touching an AP interface. It's perhaps convenient that names are (currently) equally useful for identifying an item or location because of the 1-to-1 mapping, but that name's purpose is to be a human-readable text description of the item/location. That's why I suggested "If the mod does rely heavily on item and location names being Pascal cased, I would strongly recommend changing the behavior to rely solely on IDs instead and then fixing up the names." Treat IDs and names as having mutually exclusive purposes, and you can best utilize each one. Simplifies bugfixes related to typos or string manipulation, improves UX, looks nice, etc.

But, I understand that I'm coming at this from a position where my game and its mod already identify items and locations by a numerical ID, which lets me give the AP ID meaning. It looks like your AP IDs are pretty arbitrary. And it wouldn't surprise me if there were a few spots in Emerald where I used a string somewhere for an item. So don't overcomplicate your code just to adhere to the above philosophy. I just wanted to elaborate on my suggestion.