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

Fix cartridge title name encoding (needs to be ShiftJIS string encoding) #49

Merged
merged 14 commits into from
May 8, 2021

Conversation

binary1230
Copy link
Collaborator

WIP, needs a little more testing before it's ready.

fix shift-JIS encoding for reading cart titles

- need to read using ASCI encoding of Shift-JIS (Ascii + Japanese Chars)
- add unit tests

thanks to SNESLAB Discord and @LuigiBlood for the fix!
…ield

- need to read using ASCI encoding of Shift-JIS (Ascii + Japanese Chars)
- add unit tests
- NOT quite finished yet.

thanks to SNESLAB Discord and @LuigiBlood for the fix!
- padding algorithm better
- other quality of life improvements
- more tests!
@binary1230 binary1230 changed the title Fix cart title encoding Fix cartridge title name encoding (needs to be ShiftJIS string encoding) May 4, 2021
@binary1230
Copy link
Collaborator Author

binary1230 commented May 4, 2021

All the stuff related to the cart title SHOULD be working now.

However, couple things left:

  • Make the verification error that pops up be a skippable warning 'if you know what you're doing click here'
  • Write unit tests for checksum bytes for both Hi and Lo roms. I think we've been wrongly assuming a HiRom offset for all ROMs, when we should be picking different offsets for different rom types.
  • Test 2 byte characters for ROM cartridge names, make sure they still equal 21 BYTES (not 21 CHARS)

Diz.Core/SampleRomData.cs Show resolved Hide resolved
Diz.Test/CartTitleTests.cs Show resolved Hide resolved
@@ -30,7 +30,7 @@ private void InitializeComponent()
{
System.ComponentModel.ComponentResourceManager resources = new System.ComponentModel.ComponentResourceManager(typeof(ExportDisassembly));
this.cancel = new System.Windows.Forms.Button();
this.button2 = new System.Windows.Forms.Button();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is an unrelated change that crept in here. normally i'd remove it but, it won't hurt anything (just a rename)

- copy some checksum code from Asar
- build external test runner for running external tools in unit tests
- hook up SuperFamiCheck external tool to print checksum/complement data
- write Sprache parsers to parse stdout from SuperFamiCheck
- refactor out all ROM import setting code into utility class
- write unit tests to compare checksums via several methods now, all seems to be working
- various other cleanup
- we'll automatically fix detected bad title string data in older save games
- add hooks for units tests
- write full unit test suite for this bug
- add partial support for mitigations on load/save and test scaffold
- fix some bugs with SampleData
- more general cleanup
- add unit tests for Bug #50 and associated infrastructure
- further heavy refactor of linked ROM code. this should be pretty much final
- add a ton of unit tests
- add near-100% test coverage for migration code
@binary1230 binary1230 marked this pull request as ready for review May 8, 2021 20:50
- these were added by mistake, shoudn't be present outside the test .csproj
@@ -2,7 +2,6 @@
using System.ComponentModel;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note for future people: this form contained a lot of controller / business logic, in this PR it's been refactored out so it can more easily be unit tested.

- woops, FluentValidation is actually part of the runtime (not just for tests), my bad
@binary1230
Copy link
Collaborator Author

this is pretty thoroughly tested (and somewhat overkill on the testing hah)

This PR ended up being heavy on refactoring out a few subsystems to make future fixes easy, and allow load/save/import functionality to be unit tested. it should make things much simpler in the future to deal with.

I spun off a few low-pri issues that I didn't want to hold up getting a release out for, they are:
#51 #52 #53

Thanks @LuigiBlood for the input and Sneslab discord for info on a few things in here.

@binary1230 binary1230 merged commit 15dd25e into master May 8, 2021
@binary1230 binary1230 deleted the fix_cart_title_encoding branch May 8, 2021 21:40
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.

None yet

1 participant