LADX: Improve icon guesses for foreign items

[threeandthreee]

What is this fixing or adding?

There already exists a system for assigning icons to foreign items based on their name. This fleshes out the list of forbidden words and synonyms to cover a wider range of items and moves the list to a new file (because its long).

How was this tested?

Packed adjustments into ladx.apworld and playtested.

@zig-for

As an aside, I'd like to use some other icons such as the piece of power for triforce pieces, but not sure how to do that yet :)

[zig-for]

🙏 Thank you for this, a few questions. I agree that we should figure out how to get pieces of power working.

[threeandthreee]

Got pieces of power working, as well as guardian acorns. No animation on the piece of power though, haven't been able to figure that out.

I think it should also be possible to place a bomb arrow, which would be a fun one to use for missiles.

[zig-for]

Excellent. Animations on pieces of power probably won't "just work". I'll poke at it, but we might need to flag down daid.

[zig-for]

See chest.asm, I think the code in use for renderLargeItem also needs to be called if you pick up a piece of power.

[threeandthreee]

See chest.asm, I think the code in use for renderLargeItem also needs to be called if you pick up a piece of power.

The code for color cycling the instruments? I tried that and it didn't seem to do anything. As I understand it, the piece of power should be swapping between two variants rather than color cycling.

I think that this is the piece of power animation code in the disassembly https://github.com/zladx/LADX-Disassembly/blob/main/src/code/entities/bank3.asm#L3024

My attempts to incorporate the code so far have just led to failures to generate, but I'm brand new to assembly.

[zig-for]

I can give it a shot later

[zig-for]

Bump, is this ok even without anims @threeandthreee

[threeandthreee]

Yeah, animation is just an extra as far as I'm concerned.

[threeandthreee]

Just to check, is there anything else I can do toward getting this pull request merged?

[NewSoupVi]

Zig is no longer actively maintaining LADX.

I'm still willing to merge LADX PRs, but only with a larger volume of peer reviews.

[threeandthreee]

This could have been a separate PR but I wanted to use the Guardian Acorn and Piece of Power.

Added a yaml option to simply indicate whether or not foreign items are progression, where progression items are a Piece of Power and other items are Guardian Acorns. Went with these two since they don't apply to any local items.

Was mentioned on discord and is pretty simple to put in.
https://discord.com/channels/731205301247803413/1282851721420210318/1282856567351541790

[NewSoupVi]

For Witness, there are three major item groups

Every item in the "Doors" item group should resolve to a Key
Except for "Boat", which should be, like the angler hook or flippers or sth idk

Every item in the "Lasers" item group could resolve to... well, a random instrument, tbh? Since that's the equivalent, it's the "main McGuffin".

Off of vibes, without thinking about it too much (feel free to suggest sth different):
Symmetry Laser - Triangle
Desert Laser - Marimba
Quarry Laser - Drums
Shadows Laser - Violin or Harp?
Keep Laser - Organ
Town Laser - Bell
Monastery Laser - Violin
Jungle Laser - Harp or Marimba?
Bunker Laser - Harp (idk, it's a greenhouse full of flowers)
Swamp Laser - Horn
Treehouse Laser - Horn

Every item in the Symbols item group could probably just be the map. Although some of them could have better associations, it's just hard to find one for all of them

Finally, there are the "Puzzle Skip" (useful), "Speed Boost" (filler), "Power Surge" (trap), "Slowness" (trap) and "Bonk" (trap) items.
Puzzle Skip can probably be sth like the Gold Leaf or Acorn.
Speed Boost imo should just look like a Rupee or sth. Alternatively, it could be the Pegasus Boots.
The traps could all just be... a bomb? Or maybe some sort of progression item for the bait:tm:

[NewSoupVi]

Ftr, I believe the door group would be covered by the following phrases:

"Door"
"(Door)"
"Panel"
"(Panel)"
"Doors"
"Panels"
"Water Pumps"
"Gates"

[threeandthreee]

It already pluralizes (well, sticks 's' at the end), so 'DOOR' will work for both 'DOOR' and 'DOORS'. Off the top of my head I can't think of what it does with parens, but would make sense to filter those out too.

[NewSoupVi]

I realized there's also a couple Door items called "Shortcuts"

#2201 (comment)

But after that I think it's truly all door items accounted for, I had a quick glance over the full list

[SushiKishi]

Seeds with this implemented have generated as expected.

I have personally only used the progression/non-progression setting with this code implemented, so I can't comment on the "name => icon" selection side of this change. 

I was going to suggest that the PoP icon should blink, as the 'vanilla' one does, but it leads back to confusion -- is that a Piece of Power, or is it an AP item drop? Having it non-blinking makes it obvious which is which.

I'd rather just get, I dunno, a star like in LTTP, but I realize options are limited within the confines of a GBC cart, and if there were no further additions to the feature I'd still be using the Progression/Non-Progression option 100% of the time.

[threeandthreee]

In vanilla LADX, when you find a small key that drops from the ceiling, you get no text box when you pick it up. The randomizer has kept this behavior but its a problem when we're customizing foreign items because if it looks like a small key, its treated as a small key, so the text box can get skipped when it shouldn't be.

Adding a change to this branch that removes the handling to skip text on small key drops. We polled the channel and the majority is in favor of this.

https://discord.com/channels/731205301247803413/1090819435893362768/1300544655435370607

[threeandthreee]

I was going to suggest that the PoP icon should blink, as the 'vanilla' one does, but it leads back to confusion -- is that a Piece of Power, or is it an AP item drop? Having it non-blinking makes it obvious which is which.

An attempt was made earlier, I couldnt figure out the animation 😄

I'd rather just get, I dunno, a star like in LTTP, but I realize options are limited within the confines of a GBC cart, and if there were no further additions to the feature I'd still be using the Progression/Non-Progression option 100% of the time.

A custom icon would certainly be possible but its not something I'm personally looking to work on

[palex00]

This PR was tested as part of a Beta apworld that was used in three dedicated Test Asyncs, as well as dozen of normal Syncs. I have also personally run 200 test generations which succeeded.

We have also done polling on what users want, etc. which can be found here

The new feature has worked as expected and other stuff does not seem impacted.

[NewSoupVi]

Seems good. Did a bunch of generations together with the async filler yaml.

I have a couple of minor comments about code quality, but they're not that important - Mostly putting these here because I know threeandthree is an aspiring world maintainer so I want to give them some learning opportunities

Anyway, I'll probably just merge this soon:tm:, unless there are any objections. More of these associations can be added in future PRs and they should be much simpler.

[NewSoupVi]

I forgor lol