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

Expand cartridge information #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fxck-tony
Copy link

Expand PyBoy API to return more data from the ROM cartridge's header as it region and cartridge type

@@ -15,6 +15,8 @@ cdef Logger logger
cdef class BaseMBC:
cdef str filename
cdef str gamename
cdef srt gametype
Copy link
Owner

Choose a reason for hiding this comment

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

Misspelled str

Comment on lines +110 to +116
def getgametype(self, rombanks):
if rombanks[0, 0x143] == 0x80 or rombanks[0, 0x143] == 0xc0:
return "Game Boy Color"
elif rombanks[0, 0x146] == 0x03:
return "Super Game Boy"
else:
return "Game Boy"
Copy link
Owner

Choose a reason for hiding this comment

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

It's probably better to split this out into separate "cgb" and "sgb" checks, as they are seemingly not mutually exclusive https://gbdev.io/pandocs/The_Cartridge_Header.html

else:
return "Unknown"
# Return game region according to the last letter of the serial code
if region_code == 'J':
Copy link
Owner

Choose a reason for hiding this comment

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

Where did you find these codes?

game_type = self.getgametype(rombanks)
# The region character is only present in GBC/SGB games only; for regular GB games can't be fetched
if game_type == 'Game Boy':
return "Unknown"
Copy link
Owner

Choose a reason for hiding this comment

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

It would be better to return None, when we don't have an answer. It's more intuitive that it returns something that evaluates to False: bool(pyboy.cartridge_region) == False.

I.e. this won't work if it's "Unknown":

if python.cartridge_region:
    do_something(python.cartridge_region)

Comment on lines +326 to +361
self.cartridge_type = self.mb.cartridge.gametype
"""
The game type stored on the currently loaded cartridge ROM. Values are:
Game Boy, Game Boy Color, Super Game Boy

Example:
```python
>>> pyboy.cartridge_type # Game type of PyBoy's default ROM
'Game Boy Color'

```

Returns
-------
str :
Game type
"""

self.cartridge_region = self.mb.cartridge.gameregion
"""
The game region stored on the currently loaded cartridge ROM. Example values are:
Europe, USA, Japan, Spain, Germany, World...

Example:
```python
>>> pyboy.cartridge_region # Game region of PyBoy's default ROM
'Europe'

```

Returns
-------
str :
Europe
"""

Copy link
Owner

Choose a reason for hiding this comment

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

I think it might make sense to move this to pyboy.cartridge.region and create a cartridge API, just like pyboy.screen etc. I can do it, if you think it's too much.

@@ -103,7 +105,55 @@ def init_rambanks(self, n):
self.rambanks = memoryview(array.array("B", [0] * (8*1024*16))).cast("B", shape=(16, 8 * 1024))

def getgamename(self, rombanks):
return "".join([chr(rombanks[0, x]) for x in range(0x0134, 0x0142)]).split("\0")[0]
return "".join([chr(rombanks[0, x]) for x in range(0x0134, 0x0143)]).split("\0")[0]
Copy link
Owner

Choose a reason for hiding this comment

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

Would it make sense to conditionally extract a shorter or longer string if it's a CGB or DMG ROM? I know it's a little outside of your PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants