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

Terraria: Implement New Game #1405

Merged
merged 85 commits into from
Jul 19, 2023
Merged

Terraria: Implement New Game #1405

merged 85 commits into from
Jul 19, 2023

Conversation

Seldom-SE
Copy link
Collaborator

@Seldom-SE Seldom-SE commented Jan 23, 2023

What is this fixing or adding?

Terraria world randomization

How was this tested?

Manual verification and existing unit tests. Also, the parser checks for many errors in the data file (syntax errors, unrecognized logic functions, unrecognized item names, and duplicate items/locations/goals/flags/etc).

Some info to help the review process (see this)

People who have worked on this: just me (Discord @seldom_se)
Design thread
Current apworld

I think this message and the next several messages following it show the current state of this integration. Most people don't run into any bugs, but some do, though they tend to be on the client-side. I occasionally tweak the logic for balance and fixes, and to account for discovered skips.

Calamity support is nearly done, but I'll save that for another pr. There are quite a few known bugs on the client at the moment.

I'll keep this comment up-to-date with the current state of the integration until it's merged

worlds/terraria/Options.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@espeon65536 espeon65536 left a comment

Choose a reason for hiding this comment

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

A short section about compatibility with other mods should also be included in the setup guide. I imagine that any mod that significantly alters gameplay progression (Calamity etc.) would be unsafe to use, while QoL mods (Veinminer etc.) should be fine.

worlds/terraria/Checks.py Outdated Show resolved Hide resolved
worlds/terraria/__init__.py Outdated Show resolved Hide resolved
worlds/terraria/Options.py Outdated Show resolved Hide resolved
worlds/terraria/Checks.py Outdated Show resolved Hide resolved
worlds/terraria/docs/setup_en.md Outdated Show resolved Hide resolved
worlds/terraria/docs/setup_en.md Outdated Show resolved Hide resolved
@Seldom-SE Seldom-SE requested a review from espeon65536 January 23, 2023 10:36
operator, #: bool | None,
conditions, #: list[tuple[bool, int, str | tuple[bool | None, list], str | int | None]],
) -> bool:
if operator is None:
Copy link
Member

Choose a reason for hiding this comment

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

This feels like it could be done with assert, to be skipped when shipped, as this should only raise during dev, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, operator is allowed to be None when there's only one condition in the group. Like if some conditions are just Wall of Flesh, no & or | is provided, so operator is None. As opposed to Wall of Flesh & Plantera, where operator would be false to represent &

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.

In addition to the following, don't forget to also add "Terraria" to the list of supported games in the root README.md file.

worlds/terraria/Options.py Outdated Show resolved Hide resolved
worlds/terraria/Options.py Outdated Show resolved Hide resolved
worlds/terraria/__init__.py Outdated Show resolved Hide resolved
worlds/terraria/__init__.py Outdated Show resolved Hide resolved
worlds/terraria/__init__.py Outdated Show resolved Hide resolved
worlds/terraria/__init__.py Outdated Show resolved Hide resolved
worlds/terraria/__init__.py Show resolved Hide resolved
worlds/terraria/Checks.py Outdated Show resolved Hide resolved
worlds/terraria/docs/setup_en.md Outdated Show resolved Hide resolved
worlds/terraria/docs/setup_en.md Outdated Show resolved Hide resolved
@ThePhar ThePhar changed the title Terraria: implement new game Terraria: Implement New Game Jul 19, 2023
@ThePhar ThePhar merged commit 83387da into ArchipelagoMW:main Jul 19, 2023
@Seldom-SE Seldom-SE deleted the terraria branch July 19, 2023 16:02
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Co-authored-by: Zach Parks <[email protected]>
Co-authored-by: Fabian Dill <[email protected]>
kl3cks7r pushed a commit to kl3cks7r/Archipelago that referenced this pull request Dec 15, 2023
Co-authored-by: Zach Parks <[email protected]>
Co-authored-by: Fabian Dill <[email protected]>
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Co-authored-by: Zach Parks <[email protected]>
Co-authored-by: Fabian Dill <[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.

7 participants