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

Pokémon R/B: Migrate support into Bizhawk Client #2466

Merged
merged 39 commits into from
Nov 25, 2023

Conversation

Alchav
Copy link
Contributor

@Alchav Alchav commented Nov 15, 2023

What is this fixing or adding?

  • Removes the Pokémon Client, adding support for Red and Blue to the Bizhawk Client.
  • Adds /bank commands that mirror SDV's, allowing transferring money into and out of the EnergyLink storage.
  • Adds a fix to the base patch so that the progressive card key counter will not increment beyond 10, which would lead to receiving glitch items. This value is checked against and verified that it is not > 10 as part of crash detection by the client, to prevent erroneous location checks when the game crashes, so this is relevant to the new client (although shouldn't happen unless you're using !getitem, or putting progressive card keys as item link replacement items)

How was this tested?

By testing it. A client has also been made available in #pkmn-red-blue for further testing by others.

Zunawe and others added 30 commits July 8, 2023 02:53
Bad commit organization a consequence of working with two different branches and not keeping the commits separated
…perations

On 2.9, it would detect LuaJIT and flood the console with deprecation warnings
Also splits out BizHawk communication functions to their own file for use outside this client
# Conflicts:
#	inno_setup.iss
#	worlds/_bizhawk/__init__.py
#	worlds/_bizhawk/client.py
#	worlds/_bizhawk/context.py
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.

Lots of single quoted strings that should be double quoted.

If this is a supported way to dynamically add or remove commands

ctx.command_processor.commands["bank"] = cmd_bank

then there's probably a reasonable interface that could be added to BizHawkClient where you define your commands on a classvar and the main loop will add and remove your commands when it detects ROM changes.

As it is though, I don't think CommonClient is built in a way that expects dynamically added commands, so it'd be better to start from there and work down. I think this method is fine for now, since it will only be present if you switch games after playing R/B and try to use it, and it checks you and tells you what you did wrong.

inno_setup.iss Outdated Show resolved Hide resolved
worlds/LauncherComponents.py Outdated Show resolved Hide resolved
worlds/pokemon_rb/client.py Outdated Show resolved Hide resolved
worlds/pokemon_rb/client.py Show resolved Hide resolved
original_money = original_money[0]
original_money = [int(byte) for byte in original_money]
money = [int(hex(byte).split("x")[1]) for byte in original_money]
money = (money[0] * 10000) + (money[1] * 100) + money[2]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks really weird. I'll believe that it's a silly representation that GF used either to store large numbers or to "encrypt" the number, but int(hex(byte).split("x")[1] looks especially hacky. A quick comment explaining what's going on would be warranted imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's binary-coded decimal, where 0x99 means 99.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems complicated. Can't you just do money = int(original_money[0].hex())?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems complicated. Can't you just do money = int(original_money[0].hex())?

int(original_money.hex()) works. I was not aware .hex() on a bytes object would do this.

worlds/pokemon_rb/client.py Outdated Show resolved Hide resolved
worlds/pokemon_rb/client.py Outdated Show resolved Hide resolved
@Zunawe
Copy link
Collaborator

Zunawe commented Nov 16, 2023

Only just noticed this was a draft 😅. Didn't mean to be over-eager. The discord feed didn't show draft at all, so I just clicked it and looked around.

@Alchav
Copy link
Contributor Author

Alchav commented Nov 16, 2023

Only just noticed this was a draft 😅. Didn't mean to be over-eager. The discord feed didn't show draft at all, so I just clicked it and looked around.

Draft just means I don't want it merged yet, not that I don't want help.

@Alchav Alchav marked this pull request as ready for review November 16, 2023 04:30
@ThePhar ThePhar added the is: refactor/cleanup Improvements to code/output readability or organizization. label Nov 16, 2023
if location.ram_address is not None:
if type(location.ram_address) == list:
location_map[type(location.ram_address).__name__][(location.ram_address[0].flag, location.ram_address[1].flag)] = location.address
location_bytes_bits[location.address] = [{'byte': location.ram_address[0].byte, 'bit': location.ram_address[0].bit},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still a bunch of single-quoted strings here and there that should be double-quoted

@Berserker66
Copy link
Member

Note that InnoSetup will not delete old files, like the old .exe or the old .lua unless instructed to do so. I did not see any such instructions. I'd strongly recommend that this is added as users are likely to just try to use the old files if they exist.

@Berserker66 Berserker66 merged commit 8a852ab into ArchipelagoMW:main Nov 25, 2023
12 checks passed
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
- Removes the Pokémon Client, adding support for Red and Blue to the Bizhawk Client.
- Adds `/bank` commands that mirror SDV's, allowing transferring money into and out of the EnergyLink storage.
- Adds a fix to the base patch so that the progressive card key counter will not increment beyond 10, which would lead to receiving glitch items. This value is checked against and verified that it is not > 10 as part of crash detection by the client, to prevent erroneous location checks when the game crashes, so this is relevant to the new client (although shouldn't happen unless you're using !getitem, or putting progressive card keys as item link replacement items)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants