-
Notifications
You must be signed in to change notification settings - Fork 475
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
base: master
Are you sure you want to change the base?
Expand cartridge information #336
Conversation
@@ -15,6 +15,8 @@ cdef Logger logger | |||
cdef class BaseMBC: | |||
cdef str filename | |||
cdef str gamename | |||
cdef srt gametype |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misspelled str
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" |
There was a problem hiding this comment.
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': |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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)
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 | ||
""" | ||
|
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
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
Expand PyBoy API to return more data from the ROM cartridge's header as it region and cartridge type