-
Notifications
You must be signed in to change notification settings - Fork 500
Metroid ii wrapper #362
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
base: master
Are you sure you want to change the base?
Metroid ii wrapper #362
Conversation
…figure out why the values are diff from in game
…o calculate things like health, and possibly add a more human readable upgrade field rather than just using the games constants
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.
Great work! The comments are mostly just clean up
|
||
|
||
cdef class GameWrapperMetroidII(PyBoyGameWrapper): | ||
cdef readonly int _game_over |
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.
readonly
makes it available on the API. No need to have this, when we have the game_over
method as well.
|
||
def __init__(self, *args, **kwargs): | ||
self._game_over = False | ||
# may need to change game area section. Copied from the kirby code |
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.
Can you check out what the game area should be?
# Access RAM for a ton of information about the game | ||
# I'm getting all of the values from datacrystal | ||
# https://datacrystal.tcrf.net/wiki/Metroid_II:_Return_of_Samus/RAM_map | ||
# I've skipped some of the values like music related ones | ||
|
||
# Constants from decomp project | ||
# https://github.com/alex-west/M2RoS/blob/main/SRC/constants.asm |
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.
Maybe condense this down to:
# RAM addresses and constants sourced from:
# https://datacrystal.tcrf.net/wiki/Metroid_II:_Return_of_Samus/RAM_map
# https://github.com/alex-west/M2RoS/blob/main/SRC/constants.asm
# this could be done much faster by checking health | ||
# There's a death animation so the extra few seconds over thousands of | ||
# runs *could* add up? |
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.
Then shouldn't we use this method instead?
# X position within area (pixels/screen) | ||
self.x_pos_pixels = self.pyboy.memory[0xD027] | ||
self.y_pos_pixels = self.pyboy.memory[0xD029] | ||
self.x_pos_area = self.pyboy.memory[0xD028] | ||
self.y_pos_area = self.pyboy.memory[0xD02A] |
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.
What's the difference between "pixels" and "area"?
Kwargs: | ||
* timer_div (int): Replace timer's DIV register with this value. Use `None` to randomize. | ||
""" | ||
# TODO implement me, I don't know if I really need to do anything here? |
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.
Should be fine as is. We can remove the comment
def __init__(self, *args, **kwargs): | ||
self._game_over = False | ||
# may need to change game area section. Copied from the kirby code | ||
super().__init__(*args, game_area_section=(0, 0, 20, 16), game_area_follow_scxy=True, **kwargs) |
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.
Could you add all the attributes you've added here and give them a default value? And underneath each attribute add a docstring?
```text | ||
0 1 2 3 4 5 6 7 8 9 | ||
____________________________________________________________________________________ | ||
0 | 383 383 383 301 383 383 383 297 383 383 383 301 383 383 383 297 383 383 383 293 | ||
1 | 383 383 383 383 300 294 295 296 383 383 383 383 300 294 295 296 383 383 299 383 | ||
2 | 311 318 319 320 383 383 383 383 383 383 383 383 383 383 383 383 383 301 383 383 | ||
3 | 383 383 383 321 322 383 383 383 383 383 383 383 383 383 383 383 383 383 300 294 | ||
4 | 383 383 383 383 323 290 291 383 383 383 313 312 311 318 319 320 383 290 291 383 | ||
5 | 383 383 383 383 324 383 383 383 383 315 314 383 383 383 383 321 322 383 383 383 | ||
6 | 383 383 383 383 324 293 292 383 383 316 383 383 383 383 383 383 323 383 383 383 | ||
7 | 383 383 383 383 324 383 383 298 383 317 383 383 383 383 383 383 324 383 383 383 | ||
8 | 319 320 383 383 324 383 383 297 383 317 383 383 383 152 140 383 324 383 383 307 | ||
9 | 383 321 322 383 324 294 295 296 383 325 383 383 383 383 383 383 326 272 274 309 | ||
10 | 383 383 323 383 326 383 383 383 2 18 383 330 331 331 331 331 331 331 331 331 | ||
11 | 274 383 324 272 274 272 274 272 274 272 274 334 328 328 328 328 328 328 328 328 | ||
12 | 331 331 331 331 331 331 331 331 331 331 331 328 328 328 328 328 328 328 328 328 | ||
13 | 328 328 328 277 278 328 328 328 328 328 328 328 328 277 278 328 328 277 278 277 | ||
14 | 328 277 278 279 281 277 278 328 328 277 278 277 278 279 281 277 278 279 281 279 | ||
15 | 278 279 281 280 282 279 281 277 278 279 281 279 281 280 282 279 281 280 282 280 | ||
``` |
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.
Remember to update this view, as this is copied from Kirby.
Even better, you can make it a test as in the Super Mario Land wrapper (without the mapping):
>>> pyboy = PyBoy(supermarioland_rom)
>>> pyboy.game_wrapper.start_game()
>>> pyboy.game_wrapper.game_area_mapping(pyboy.game_wrapper.mapping_compressed, 0)
>>> pyboy.game_wrapper.game_area()
array([[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 13, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 14, 14, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[ 0, 14, 14, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10],
[10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10]], dtype=uint32)
# yapf: disable | ||
return ( | ||
f"Metroid II:\n" + | ||
# TODO add relevant variables to print for debugging purposes |
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.
Please add the attributes you find relevant
# I don't like seeing the huge grid whne trying to develop | ||
# The sprite list can be messy too, since many things (especially | ||
# samus) are composed of multiple sprites, so simply running causes | ||
# the output to constantly change "shape", making it very difficult | ||
# to read | ||
# super().__repr__() |
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.
Although I understand, please add it back in, so each game wrapper follows the same structure.
…ver check, pixels vs area difference, check difference between displayed/current hp/missiles, verify hp calculation equations commented out, game initialzation ticks could be shrunk, view needs updating. Mostly functional/api changes for this commit
Rebasing after large update to main, only really impacted manager and manager_gen
…figure out why the values are diff from in game
…o calculate things like health, and possibly add a more human readable upgrade field rather than just using the games constants
…ver check, pixels vs area difference, check difference between displayed/current hp/missiles, verify hp calculation equations commented out, game initialzation ticks could be shrunk, view needs updating. Mostly functional/api changes for this commit
Metroid II wrapper.
Most of the essential things are there, and I can make as many changes as needed, but this should have the core functionality. The RAM mapping values were taken from Datacrystal's RAM mappings and they all seem to be valid from my testing. I only implemented the ones that seemed relevant for AI purposes. (I skipped weird music timers and pause flags.