search

Rangi42 / polishedcrystal

An upgrade to Pokémon Crystal. Brings features and content up to date, and adds some original content.
https://hax.iimarckus.org/topic/6874/
1.03k stars 196 forks source link

Analogue Pocket IPS Patch? #661

Closed msirianni closed 2 years ago

msirianni commented 2 years ago

Any chance we could get an IPS patch or IPS patched version so that this great ROM can be used on the new Analogue Pocket GB Studio Mode?

Instructions are here: https://github.com/JoseJX/analogue-pocket-patches/blob/main/README.md#how-theyre-generated

FredrIQ commented 2 years ago

It should be possible to create a custom hardware_constants.asm adapted for the Analogue Pocket... but why is this necessary? I thought this device was supposed to be reasonably accurate?

An IPS patch isn't very feasible as a format here since this project is essentially handled as an open source project.

msirianni commented 2 years ago

This would allow Polished Crystal to be played from the Analogue Pocket's SD card as homebrew without the need for a flash cart. Perhaps an alternate release would be more appropriate than a patch?

FredrIQ commented 2 years ago

Commit ee974d5 (part of 9bit, which isn't yet released publically and needs to be built from source) addresses most of the parts needed. Rangi (who knows the makefile better than I do) should be able to do the final touch-ups needed to make it straightforward to build a .pocket file for Analogue Pocket benefit.

That being said, I'm not sure if @Rangi42 wants to make it autobuilt like regular nightlies are at the moment once those small fixes are made? That's up to her. Keep in mind that, either way, this would take a bit of time to get up and running since 9bit isn't yet fully ready for public release anyway.

msirianni commented 2 years ago

@FredrIQ that is so thoughtful! Thank you for all the work you've done here and elsewhere. I've cloned the 9bit branch locally and am more than happy to build and test the pocket version myself if/when Rangi has completed any of the required touch-ups.

I've been excited for the 9bit branch progress anyway and this just gives me one more reason to follow it more closely. Cheers to you both!

msirianni commented 2 years ago

@FredrIQ I got your changes to build via make pocket after adding a pocket: crystal target to the Makefile and defining rSTAT_INT_DEFAULT for the "ANALOGUE_POCKET" if case in hardware_constants.asm.

I also compared the commit changes you made to ones that @zhuowei used in this Analogue Pocket patch made for vanilla Crystal which led me to copying the RGBFIX CL options into the Makefile and adding the Analogue Pocket logo to the header.asm which I believe are required to get the built ROM to run.

After a quick extension update from .gbc to .pocket I was able to see and launch the ROM on my Pocket, but immediately got an error upon launch. I plan to attempt to debug with this SameBoy fork that is patched to run .pocket files and to print warnings if/when there are register accesses to 0xFF40 - but I've got no prior assembly or ROM hacking experience so it will be a tall order! Wish me luck.

FredrIQ commented 2 years ago

Just to be clear, you did build the rom with "make pocket"? Since I do want to keep being able to build regular roms by default, you need this to tell it to build a pocket version.

Yeah I wasn't aware of the logo thing, the link you gave didn't mention it but I found another document about it but haven't yet added a commit for it. But you mentioned addressing this and it still not running correctly?

msirianni commented 2 years ago

Yes, I am building the pocket version via make pocket which failed at first due to rSTAT_INT_DEFAULT being undefined for the if DEF(ANALOGUE_POCKET) case. I added rSTAT_INT_DEFAULT EQU rSTAT_INT_MODE_0 copied from the else block but that may not have been the correct choice. I also had to define a pocket target in the Makefile (pocket: crystal).

I’m repairing my Xcode developer environment so that I can get the SameBoy fork mentioned above working to review for warnings as I have free time and will report back if I can learn anything there. I’ll definitely need to brush up on my Assembly operators and bitshifting!

FredrIQ commented 2 years ago

Could you try the last commit? 61cb856fbfde0e8d5d8952deb733524cbab06857 fixed the lack of a specialized ROM header logo for the Pocket, which I think was the last outstanding issue (besides the rSTAT_INT_DEFAULT issue which I also fixed).

msirianni commented 2 years ago

We are extremely close here, but need I think we need to find the set of RGBFIX_FLAGS that yield a ROM with the correct Header/Correct Header Checksum? Last commit you linked is outputting one with 0x4B which isn't running on the Pocket.

Logo appears to be correct now and that check is passing. Haven't noticed any memory access warnings via the SameBoy fork either.

FredrIQ commented 2 years ago

I was following the instructions in https://github.com/treyturner/analogue-pocket-patches/blob/main/TUTORIAL.md#modifying-the-header

msirianni commented 2 years ago

After examining a few of the other Pocket patches I was able to make a very slight change to line 28 of the Makefile of your most recent commit and get it running! I changed the -m 0x10 to -m 0x1b instead which is something I had seen in these other patches.

IMG_9807

Now it is working flawlessly! Thank you for your help on this. It is exciting that this will be usable with the new 9bit version as it develops.

FredrIQ commented 2 years ago

Changing MBC to 0x1B switches the ROM type to MBC5, which breaks RTC support. I don't think this should be necessary?

zhuowei commented 2 years ago

The RTC is not emulated in Analogue Pocket mode anyways (we noticed that in the Crystal patch)

FredrIQ commented 2 years ago

So building in pocket mode should probably also include the NO_RTC define and its assortment of special code to simulate an RTC. This code is, to my knowledge, completely untested and needs to be looked into more closely, but thanks for the info!

FredrIQ commented 2 years ago

Does the last commit work without needing to modify the game yourself now?

Besides the filename that is, I'll leave @Rangi42 to handle that since I don't really know how to handle that best.

(Note that you'll have to make clean first because the of the new nortc defines)

msirianni commented 2 years ago

I can confirm the latest commit builds and runs without modification after a make clean

@FredrIQ your assumption is correct! Analogue Pocket supports RTC but only when playing from cartridges (real or otherwise). When playing in the GB Studio mode it doesn't support the RTC as @zhuowei pointed out which is probably why the MBC5 type is required, so the NO_RTC define sounds like a great idea when building this way.

In addition to including that NO_RTC define it would be even easier to use if it built directly to or otherwise renamed the generated .gbc file as a .pocket file with the no more than 15 characters before the extension. I've been naming them polishedcrystal.pocket in my testing which has worked just fine. Of course it is trivial to manually rename though!

FredrIQ commented 2 years ago

Hmm I am getting conflicting information from #analogue-pocket in the Classic Gaming discord (linked above).

According to them, building a MBC3 ROM shouldn't cause it to fail to load (but it won't support the RTC either way). Yet, the game didn't run for you before I changed it to build a MBC5 ROM for Pocket? That is a bit odd. Maybe different firmware versions?

Great to know that it's working for you now though! Keep in mind that 9bit is still work in progress. It is likely that once it releases as a stable version, saves you create on your current version will not work -- because we want to add the additional PLA formes and species. This should happen very soon though, since 9bit is essentially "done", only lacking playtesting and tracking down bugs found during it.

msirianni commented 2 years ago

I changed it back to MBC3/-m 0x10 and that build is now also running so sorry for any confusion that I caused! Discord is correct that it will load as MBC3 as well. My guess is that my file versioning got mixed up and corrected at some later point during my testing and I misattributed it to the change to MBC5 that I had made.

Good to hear that 9bit is nearing a stable release. Let me know if you need any help playtesting that (or the NO_RTC code mentioned earlier).

Rangi42 commented 2 years ago

@msirianni Running make pocket should now create pkpc-3.0.0-beta.pocket, an MBC3 ROM. You should also be able to add flags like make pocket nortc noir to get their effects and still have the same 15-character filename. (Out of beta it will probably be polished-3.0.0.pocket.) Please confirm if it works!

msirianni commented 2 years ago

@Rangi42 I can confirm make pocket and the subsequently generated pkpc-3.0.0-beta.pocket worked like a charm! Thanks so much to you and @FredrIQ both for the help on this. I'm excited to play it!

JoseJX commented 2 years ago

I'm not sure if you want me to open a new bug or submit a patch, but while MBC3 .pocket ROMs do boot on the Pocket, they unfortunately don't save. It would probably be best to output an MBC5 with the NoRTC patch by default for the Pocket target. If you have any questions, I'm happy to answer them! I've been patching a lot of ROMs to .pocket format and this is unfortunately one of the limitations we've run into.

Thanks!