Baekalfen / PyBoy

Game Boy emulator written in Python
Other
4.57k stars 472 forks source link

Memory scanner utility #293

Closed NicoleFaye closed 8 months ago

NicoleFaye commented 9 months ago

Changes

Enum Classes: Two enumeration classes, CompareType and ScanMode, have been added to define the types of comparisons and scanning modes respectively.

MemoryScanner Class: This is the core class introduced in this PR. It provides methods for converting between decimal and Binary Coded Decimal (BCD) formats, and a method for scanning memory within a specified range.

Scanning and Comparison Logic: The scan_memory method allows scanning the memory between specified start and end addresses. It supports different comparison types (exact, less than, greater than, etc.) and handles both integer and BCD value types.

Implementation Details

The MemoryScanner class takes a pyboy instance in its constructor, linking the scanner directly to the emulator's memory.

Conversion methods (_dec_to_bcd and _bcd_to_dec) facilitate handling BCD values, which are common in older systems emulated by PyBoy.

The scan_memory method iterates over the specified memory range, applying the chosen comparison type to find matches for the target value. It supports scanning for both integer and BCD formatted values.

A private method _check_value is used to streamline the comparison logic within the scan_memory method.

NicoleFaye commented 9 months ago

After Creating the PR I now realize it needs a way to work with ints larger than 8 bits, so I will work on that portion now as well.

The score for pokemon pinball seems to be stored in several bytes with each byte being a BCD representation of 2 digits of the number. I am now working on method to easily read such values in BCD or integer representation and modes for big and little endian.

Baekalfen commented 9 months ago

Honestly, very nice work! :)

Really good insight to include BCD. Also, you can use bint for boolean values.

NicoleFaye commented 9 months ago

Enhanced Memory Scanning and BCD Conversion Support

Overview:

This Pull Request introduces significant enhancements to the MemoryScanner class and BCDConverter class within the utils.py module. It adds support for reading and comparing multi-byte values in memory, considering different endian types. Additionally, the BCD conversion methods have been improved for handling multi-byte BCD values with endian awareness.

Key Changes:

Extended Multi-byte Support in MemoryScanner:

The scan_memory method now supports scanning for values larger than a single byte, accommodating both little and big endian types. A new method, read_memory, has been added to encapsulate the logic for reading multi-byte values from memory, respecting the specified endian type and value type (INT or BCD).

Enhanced BCDConverter Methods:

The dec_to_bcd and bcd_to_dec methods now handle multi-byte BCD values and are endian-aware, enhancing their applicability for diverse use cases.

Limitations:

Values around int64 max may not behave as expected.

Test Cases for BCD Conversion:

Comprehensive test cases have been added in tests/test_bcd_conversion.py to ensure the correctness and reliability of the BCD conversion methods in various scenarios.

Files Modified:

utils.py: Added enhanced methods for MemoryScanner and BCDConverter. tests/test_bcd_conversion.py: New test cases for validating BCD conversion logic.

NicoleFaye commented 9 months ago

72 is the issue this PR is intended to close

NicoleFaye commented 9 months ago

Found a variable in one method was compiling to 32 bit when it needed to be 64 bit, all should work as intended now.

Baekalfen commented 9 months ago

I was reviewing your code, and to make the tests etc. work, I started changing a few things. Sorry about that. I'll let you take it from here.

I couldn't get the last part for the test_memoryscanner to work. Seems like it's not narrowing down the value. Or maybe I'm assuming something wrong.

I'd recommend you install pre-commit. It'll take care of formatting etc. when you commit your code

NicoleFaye commented 9 months ago

I don't mind you changing anything, its your repo lol.

How can I get the new test working? My pytest just skips it. Do I need to move the rom to a specific folder/rename it? I think once it passes tests it might be done.

Baekalfen commented 9 months ago

I don't mind you changing anything, its your repo lol.

I just don't want you to feel I'm taking over your PR from you 😅

How can I get the new test working? My pytest just skips it. Do I need to move the rom to a specific folder/rename it? I think once it passes tests it might be done.

I should document this somewhere. I'm fixing something in #297 and then it's just:

  1. Make directory in root of PyBoy repo called "test_roms/secrets"
  2. Place ROMs here
Baekalfen commented 9 months ago

I added a simple Wiki page about testing. I would love to hear your thought on it, and verify that it works: https://github.com/Baekalfen/PyBoy/wiki/Run-Pytests

NicoleFaye commented 9 months ago

Okay i think i messed something up im not sure, i woke up and fetched the branch, and it told me to commit a bunch of stuff i didnt write, which i think was the force push? Anyway, i committed them. But all the tests are yelling at me about _rendering not being an attribute. Do you get that when you try running the tests or do they pass? The conversion tests pass but i think thats because they dont need a pyboy instance.

image

Baekalfen commented 9 months ago

Okay i think i messed something up im not sure, i woke up and fetched the branch, and it told me to commit a bunch of stuff i didnt write, which i think was the force push? Anyway, i committed them. But all the tests are yelling at me about _rendering not being an attribute. Do you get that when you try running the tests or do they pass? The conversion tests pass but i think thats because they dont need a pyboy instance.

That's a mistake on my part. It was actually a mistake on v2.0.0, that I rebased onto your branch to get you up-to-date. It's fixed now

NicoleFaye commented 9 months ago

I can't get some of the tests working but i did get these working. Once i took out the breakpoint. Is it a complete test though? Looking at it more it looks like i may have just removed them preemptively?

Most if of the failed tests are from me missing things though i believe.

image

Also there are so many calls to pyboy.screen.image.show() in all the tests, I did the steps on the testing page on the wiki you made and it gave me a startle when what felt like 100 images opened up.

Baekalfen commented 9 months ago

Also there are so many calls to pyboy.screen.image.show() in all the tests, I did the steps on the testing page on the wiki you made and it gave me a startle when what felt like 100 images opened up.

Hmm. Weird you got those to appear. I assume it's some debug images to show how the result differed from the expected result. Try doing: TEST_CI=1 python3 -m pytest tests/. That should hide them. I'll probably invert that logic, as to make it easier for other to test. But I wonder why you see those.

NicoleFaye commented 9 months ago

Hmm. Weird you got those to appear. I assume it's some debug images to show how the result differed from the expected result. Try doing: TEST_CI=1 python3 -m pytest tests/. That should hide them. I'll probably invert that logic, as to make it easier for other to test. But I wonder why you see those.

I am on windows, which is the cause of most of my problems in life. is TEST_CI an environment var i need?

Baekalfen commented 9 months ago

I am on windows, which is the cause of most of my problems in life. is TEST_CI an environment var i need?

I guess the tests should work on Windows? They do on the CI at least... Yes, it's an environment variable to silence the debugging

NicoleFaye commented 9 months ago

The env var didnt seem to change anything for me. These are the results im getting from tests, and im sure alot of it has to do with the fact i only have the kirby rom.

image

Baekalfen commented 8 months ago

im sure alot of it has to do with the fact i only have the kirby rom.

Yes, you can see in conftest.py which files the tests expect