Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lingo: New game #1806

Merged
merged 170 commits into from
Nov 8, 2023
Merged

Lingo: New game #1806

merged 170 commits into from
Nov 8, 2023

Conversation

hatkirby
Copy link
Collaborator

What is this fixing or adding?

This is adding support for the indie puzzle game, Lingo.

How was this tested?

Many solo runs, plus a number of runs in multiworlds containing other games.

If this makes graphical changes, please attach screenshots.

hatkirby added 30 commits April 10, 2023 10:23
The connections between rooms should be fully modeled. Panel IDs are not all present. Door IDs are not present at all. Panels that aren't part of locations are not present. Doors that are not mentioned are assumed to be vanilla.

Pilgrimage, The Colorful, The Optimistic, and the Orange Tower are special cases that will need to be handled. The victory conditions (The End and The Master) are also not configured.

No validation has been run on this file to ensure that names are internally consistent and consistent with the actual game map. YAML validation has been checked.
Doors can now have custom location names too now which is important for the doors with many requirements. I also decided that a location must have at least one panel from the room that the door is in. If this isn't the case, you have to use skip_location. To handle this, I moved some doors around and some ended up in confusing places.
Well, no doors and complex doors (all orange settings) do. Simple doors is not implemented. Colors does not work. And there are definitely issues with complex doors right now where it's assuming things involving required_door and required_room are in logic when they're not. Also a lot of stuff is hardcoded when it shouldn't be.
Shuffle colors does generate now, but it is facing the same problems as shuffle doors.
I blame Python for being weird. Complex door shuffle seems to work well now! I also write unit tests to verify this behaviour.
I haven't validated this against the mapfile yet. I will probably write a script to do it and include it in this repo.
Also fixed a couple of config issues that were found by the validator.
It feels messy sending this as slot data, but I also don't love the idea of duplicating data by copying the config file into the client.
- WARTS in the directional gallery could be a check
- with the color barriers, some of them could still be vanilla
- some of the wise puzzles are half black
- is something up with the rhyme room 2 entrance?
- the bearer isn't a check
- undistracted should be a check
- one of the hide/seeks isn't actually in the hedge maze, it's in outside the agreeable
- if you solve ONE ROAD MANY TURNS from the back, you don't actually get access to orange tower third floor
- is the traveled not a check?
- rhyme room -- smiley to target
- The undeterred
- Stairs in the observant
Refactored a bunch of the player-specific logic. Events are now made in non-door-shuffle mode to speed up generation. The Optimistic and The Master now function properly (using events).
The way we were hard-coding the pilgrimage before was causing the Pilgrim Room to always be accessible if you have access to Outside The Agreeable. Now we are hard-coding it in a different and weirder way.
Red and blue are also required for the Bearer shortcut (assuming one-way).
Copy link
Collaborator

@alwaysintreble alwaysintreble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not really a fan of the use of "static classes" since it isn't really a thing in python, and the exact same thing can be accomplished by making them module level but they don't look problematic to me anymore at least. They're highly volatile and future changes on it will have to be pretty thoroughly tested, which is why I don't like them.

worlds/lingo/static_logic.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Zunawe Zunawe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Reorganizing imports to be ordered external module, AP module, lingo module would be nice. I think PEP8 recommends/requires them to be ordered anyway.

Someone else might want to comment on the use of LogicMixIn. I've been told it should be avoided and doesn't currently solve any problems that can't be solved with a normal helper function, but that's only hearsay for me, and I didn't scrutinize its use here.

Otherwise, mostly minor things. The "static" stuff is better than it was. Like treble said though, I don't think the current implementation has any benefits over doing all of this outside of any sort of class. You can just write and run an initialization function when the module gets imported and then other files can just do from .static_logic import PANELS or whatever instead of going through the LingoWorld class. Someone could still mistakenly create a second instance of StaticLingoLogic. But I personally wouldn't block the PR based on how it currently works.

Probably will approve after these are addressed/answered.

worlds/lingo/utils/assign_ids.rb Show resolved Hide resolved
worlds/lingo/Options.py Outdated Show resolved Hide resolved
worlds/lingo/player_logic.py Outdated Show resolved Hide resolved
worlds/lingo/player_logic.py Outdated Show resolved Hide resolved
worlds/lingo/static_logic.py Outdated Show resolved Hide resolved
worlds/lingo/static_logic.py Outdated Show resolved Hide resolved
worlds/lingo/static_logic.py Outdated Show resolved Hide resolved
worlds/lingo/static_logic.py Outdated Show resolved Hide resolved
worlds/lingo/static_logic.py Outdated Show resolved Hide resolved
worlds/lingo/__init__.py Show resolved Hide resolved
Copy link
Collaborator

@Zunawe Zunawe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfamiliar with the game, and only generated seeds to verify that they generate. But it looks fine on the whole. Comments I made have been acknowledged and/or addressed. But I mostly went for the low hanging fruit.

hatkirby and others added 13 commits October 16, 2023 09:51
- Cellar replica puzzles require that you have access to the Cellar.
- Lookout and Cross Tower puzzles require that you have access to the respective hint areas.
- Puzzle stacks in The Artistic require that you have access to both colors in order to solve either puzzle.

These changes are not hugely significant at the moment, but will be necessary once puzzle generation is a thing.
The first door in the starting room is now always vanilla, even with door shuffle on, and the HI panel is not a check unless panelsanity is on. The GOOD LUCK check in the second room is the new location of the good item.
@ThePhar ThePhar self-assigned this Nov 8, 2023
Copy link
Member

@ThePhar ThePhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, few issues I had were addressed in PR on their fork.

@ThePhar ThePhar merged commit ea9c313 into ArchipelagoMW:main Nov 8, 2023
12 checks passed
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Co-authored-by: Aaron Wagener <[email protected]>
Co-authored-by: Fabian Dill <[email protected]>
Co-authored-by: Phar <[email protected]>
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Co-authored-by: Aaron Wagener <[email protected]>
Co-authored-by: Fabian Dill <[email protected]>
Co-authored-by: Phar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: new game Pull requests for implementing new games into Archipelago.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants