Ziktofel / Archipelago

Archipelago Multi-Game Randomizer and Server
https://archipelago.gg
Other
1 stars 8 forks source link

Mm/item descriptions #184

Closed MatthewMarinets closed 5 months ago

MatthewMarinets commented 5 months ago

What is this fixing or adding?

Moved item descriptions from Items.py to a new file, ItemDescriptions.py. It was just getting harder to quickly scroll through the Items.py data. The data is also not really used anywhere yet, so moving it to a separate file allows future using code to opt-in to using it.

Additionally, I've taken the time to add all the remaining descriptions for Protoss upgrades, SOA abilities, generic attack/armour upgrades, Zerg unit upgrades, and Kerrigan items.

Also standardized Resource Efficiency descriptions with a little helper function. Having all the resource efficiency cost reduction values in one place really snapped things into perspective -- Wraith/Predator/Diamondback/Goliath/Firebat look reasonable; Spectre/Ghost/Reaver far less so. I think the Valkyrie values can also be tuned down a little.

How was this tested?

I import this data into the icon repository. After each commit I imported the data and checked the diff on the item repository data side. I verified that only the fields I expected to change actually changed.

If this makes graphical changes, please attach screenshots.

The icon repository is already updated with all these descriptions; it's ready to be updated again on request.

https://matthewmarinets.github.io/ap_sc2_icons/

MatthewMarinets commented 5 months ago

Everything else looks good to me, though I haven't memorized all of Zik's new items.

I haven't either, a lot of these descriptions come out of GameStrings.txt with a bit of editing to keep unit names in there and early in the description. Some come from checking the upgrade structure directly where that was more convenient or the descriptions were missing from GameStrings.txt

Will fix the DT blink description in a second.

Ziktofel commented 5 months ago

Is there a way to quickly get items without description?

Maybe add a test that makes description mandatory

MatthewMarinets commented 5 months ago

Is there a way to quickly get items without description?

Maybe add a test that makes description mandatory

Right now the main source-of-truth for the item lists is still the entries in Items.py. Before, the description field was optional. It's still optional here.

I thought about adding a test, but if you're asking for it probably it's better to include here. Can also include some simple checks on the descriptions, like making sure everything ends with a .

Ziktofel commented 5 months ago

Having the field within the item itself it's easier to track what's missing the description. By putting that into an external file, it gets very hard to get what needs its description filled.

The test itself would notify that there's a missing description if a new item is added.

MatthewMarinets commented 5 months ago

Having the field within the item itself it's easier to track what's missing the description. By putting that into an external file, it gets very hard to get what needs its description filled.

The test itself would notify that there's a missing description if a new item is added.

I'd actually disagree with this, it was fairly tough to figure out what was missing a description in Items.py, especially if it was one-off. To get started with these descriptions, I had to write a quick regex parser to make sure I got all the existing ones.

If all of the entries in Items.py were only one line, then searching for description= would highlight what was missing because it would be a break in the highlight. But because the entries got long enough to span across multiple lines, it got quite hard to track them all down.

I'd say it's quite straightforward now to run the tests and the error message should tell you exactly what item is missing. It also enforces that each PR only passes checks when all added items have descriptions.