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

Pokemon Emerald: Implement New Game #1813

Merged
merged 425 commits into from
Nov 12, 2023
Merged

Conversation

Zunawe
Copy link
Collaborator

@Zunawe Zunawe commented May 16, 2023

What is this fixing or adding?

Adds Pokemon Emerald Version as a new game.

How was this tested?

Many weeks of test games by myself and the AP community. It will likely be played in a few Unsupported games before merging, as well as plenty of games outside the AP discord.

Discord thread: https://discord.com/channels/731205301247803413/1076407822692139080

"Official" beta tests:
https://discord.com/channels/731205301247803413/1106350114777333830
https://discord.com/channels/731205301247803413/756732009052635328/1095491840544870400
https://discord.com/channels/731205301247803413/1122524129514491935

If this makes graphical changes, please attach screenshots.

N/A

Credit

Luc Luc also made some contributions to the mod itself, which don't show up as commits in this PR (GH: luctowers, Discord: lucluc)
I also would like to credit pret in whatever manner seems reasonable for the decomp project and helping figure out specific technical details about the game

To fix badge items being added to bag instead of intercepted
Getting to the gym entrance requires the devon scope; I'm unsure how this ever passed
2. Put the tracker pack into packs/ in your PopTracker install.
3. Open PopTracker, and load the Pokémon Emerald pack.
4. For autotracking, click on the "AP" symbol at the top.
5. Enter the AP address, slot name, and password.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend phrasing this as "Enter the AP Server address", or maybe "... the connection address", or even "the AP connection address" as the Archipelago acronym AP isn't exactly explained in a way that makes this intuitive to a brand new user. I agree with brevity in step-by-step instructions, and think a single word add can be useful here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Updated in 977a23c

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.

I have one nitpick, but otherwise I think everything looks pretty good.

Some notes while testing (merged main on top of this during review):

  • generate_output took around 3-4 seconds per file, unsure if that can be improved, but not the worst I've seen of currently implemented worlds. Just something to keep in mind.
  • Generation appears to be deterministic as well, which is always a good sign.

If you don't mind making that one tweak, I see no reason to hold off this PR anymore.

Functions related to pokemon species and moves
"""
import time
from random import Random
Copy link
Member

Choose a reason for hiding this comment

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

Rather than (re)import the random module at runtime, I'd recommend doing the following since you're only using it for typing.

if TYPE_CHECKING:
    from random import Random

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed in c5da1ee (also needed the strings for type hints)

@Zunawe
Copy link
Collaborator Author

Zunawe commented Nov 12, 2023

generate_output took around 3-4 seconds per file, unsure if that can be improved, but not the worst I've seen of currently implemented worlds. Just something to keep in mind.

Yeah, almost all of it is just applying the base patch, creating the final patch, and writing files. Last time I checked procedure patch, it took the entire generation down to <0.5s. The other stuff in generate_output has been profiled. There's probably some stuff that could still be improved, but nothing is taking anywhere close to as much time as the patching.

@ThePhar
Copy link
Member

ThePhar commented Nov 12, 2023

Time to cause chaos.

@ThePhar ThePhar merged commit 43041f7 into ArchipelagoMW:main Nov 12, 2023
12 checks passed
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
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.

None yet

6 participants