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

[Android] Golden Sun Carpet Glitch (background elements appearing in foreground issue) #227

Open
Burlz10 opened this issue Sep 24, 2023 · 38 comments

Comments

@Burlz10
Copy link

Burlz10 commented Sep 24, 2023

On the ship to to Tolbi. Carpet appears in the foreground instead of background. See below pictures. Thank you.
IMG_3193
IMG_3192

@andymcca
Copy link

Is this with latest build?

Similar issue in mgba from a few years ago - mgba-emu/mgba#1444

The referenced commit was reverted a couple of weeks later, so it's a similar sprite rendering issue that exists in gpsp.

Please attach save file and instruction to reproduce.

@Burlz10
Copy link
Author

Burlz10 commented Sep 24, 2023

Correct. This was the latest build v0.91. I will create a save file for testing soon.
IMG_3194

@Darkstalix
Copy link

This is also happening in GS 1 Mercury lighthouse area which i cant remember cause i replayed them this xmas and also this happens on GS 2 In mars Lighthouse ice shards/blocks. Mostly if i recall any object with transparency on it it would mostly come to foreground layer
If saves and states needed i can provide since Mars lighthouse is endgame

qwewqewqe

@andymcca
Copy link

@Burlz10 @Darkstalix saves and states for both issues please!

@Darkstalix
Copy link

Darkstalix commented Sep 24, 2023

@Burlz10 @Darkstalix saves and states for both issues please!

There you go .state and .sav
This is for Mars lighthouse issue
thanks i appreciate

GOLDEN_SUN_B_AGFE00.zip

@andymcca
Copy link

These games do weird things with the OAM/BG ordering, and rely on the hardware to interpret in a certain way. So these are similar emulation bugs to the intro and dungeon ones recently fixed.

What's happening here is that a higher priority normal OBJ (but lower in the OAM priority order) consisting of a fully transparent colour should be blocking out the carpet entirely on the OBJ layers, allowing the 'lower priority' character sprite to be visible.

But gpsp sorts the OBJ/BG layers by relative priority first and then applies this order per scanline, so the above trick wont work. We could possibly get round this by forcing sprites higher in the OAM priority order to match the most recent OBJ priority, but this might break other games. So this one might need a bit of thought to fix.

@Darkstalix
Copy link

I understand and tbh if you feel like when fixing one thing that might break something else then it would be ok to leave it as it is, really. The huge issues i had last year were the sound channel missing which was fixed finally and made me really happy.
Thanks for explaining us the issues

@andymcca
Copy link

Here's the mgba Frame Inspector for GS - here the frame is being rendered as it should be -

GS1

@andymcca
Copy link

If I disable Sprite 11 (Normal 64x64 OBJ with Colour 0), then we get the behaviour we see in gpsp

GS2

@andymcca
Copy link

The Carpet is part of BG1, which appears to be a lower priority than BG2 in this frame (maybe BG1 set as priority 2?)

GS3

@andymcca
Copy link

Sprite order - Sprite 11 is Priority 1, but all others (0 - 10) are set as Priority 3, despite being lower in the OAM order and thus should be higher priority

GS4

So Sprite 11 appears to 'Mask' the lower priority BGs from the OBJ layers (despite consisting wholly of a transparent colour), allowing the lower priority OBJs to become visible, regardless of their BG-relative priority setting

I think (David may correct me) that this is difficult to emulate in gpsp due to how we currently handle the rendering. By the time we come to render Sprite 11, we've already covered the character sprites with the carpet background. Would have to detect this specific scenario and then either a) retrieve the underlying OBJs to re-render them or b) re-order the affected pixels accordingly. Maybe there's a c)? I don't know!

@andymcca
Copy link

Nanoboy Advance also had this issue previously - nba-emu/NanoBoyAdvance#99

@Darkstalix
Copy link

Darkstalix commented Sep 25, 2023

Thanks alot for even attempting on this! Takes a lot of time to work on these
I rather keep the stability gpsp has now cause for me its the best, i dont know but for some reason gpsp is faster than mgba for me especially on these games. If you see both games side by side on these cores the gpsp has a tad faster render or how to call it which always made me loving it.
Cast on both games at the same time Odyssey for example or even some spell giving attack on all 4 members
On mgba its more like accurate but slower like the the og hardware
On gpsp its faster and for fluent? i cant really explain but its waaaaay better

If you have to restart remaking the code for these small issues but break something else i would vote to keep it as it is
Has David released the fix for the summon issues? the core says that its updated , or even pretend to update every time i try from retroarch with Online updater but the issue is still there.

Personally thanks everyone trying to fix these, appreciated

@andymcca
Copy link

@davidgfnet I found this explanation from fleroviux on the gbadev Discord -

The BG-priority of a given OBJ layer pixel is essentially the max of all OBJs that intersect with this pixel, regardless whether the intersecting pixel is transparent or opaque.

So a higher priority OBJ can elevate the priority of a lower priority OBJ without actually occluding it.

Golden Sun does that. It uses a completely transparent OBJ to elevate the priority of NPCs and the PC over a BG inside one specific rectangle (carpet)

@Burlz10
Copy link
Author

Burlz10 commented Sep 26, 2023

Thanks for looking into this. If it’s too complicated a fix then it’s fine as it is. But if there is something that can be done then of course it’s much appreciated. With regard to the summon issue, like darkstalix I attempted to update the core and see if the summon animation fixed. I experienced the same issue.

@davidgfnet
Copy link
Collaborator

Checking how other emus do this. They either render all pixels from the different pipelines and pick the top 2 ones (or one if no color effects are present) or they play with some funny front to back rendering (instead of back to front which is what gpsp does). It is indeed funny business, since the game uses the PPU in an unintended way, but GS is known for hackery :)
Might take a while to get it fixed, since it's very low priority and requires some minor rewrites. Also unsure about the performance impact (should be ~0% if properly done though).
Thanks for reporting this! Seems like all emus stumble on that one :)

@andymcca
Copy link

andymcca commented Oct 2, 2023

If you have to restart remaking the code for these small issues but break something else i would vote to keep it as it is Has David released the fix for the summon issues? the core says that its updated , or even pretend to update every time i try from retroarch with Online updater but the issue is still there.

@Darkstalix just FYI, the summon issue just got fixed today, should be in tomorrow's nightly!

@Burlz10
Copy link
Author

Burlz10 commented Oct 2, 2023

Hey I updated the core and it seems to have restarted all of my games and therefore replaced all of my autosave states when started. Am I able to load the original .sav files instead and then re-enable autosave state? If so could someone explain how to do do this? Sorry for the trouble and thanks for your work on this fix.

Edit: Looking at my save file location. New saves are being saved as .srm files instead of .sav like it was before, not sure if that helps identify the issue.

@Burlz10
Copy link
Author

Burlz10 commented Oct 2, 2023

@Darkstalix did you experience this same issue as well?

@Darkstalix
Copy link

Darkstalix commented Oct 3, 2023

Yes i just checked, it looks like the gpsp creates srm now. Was there an isue with sav files?

@DerinQyu
Copy link

DerinQyu commented Oct 3, 2023

Hey I updated the core and it seems to have restarted all of my games and therefore replaced all of my autosave states when started. Am I able to load the original .sav files instead and then re-enable autosave state? If so could someone explain how to do do this? Sorry for the trouble and thanks for your work on this fix.

Edit: Looking at my save file location. New saves are being saved as .srm files instead of .sav like it was before, not sure if that helps identify the issue.

Just rename your .sav to .srm it should work.

@Darkstalix
Copy link

Thats ok tbh if we have to go with srm from now on its ok, it doesnt matter. Im just wondering if it was on purpose or something happened by mistake on this hotfix of the core

@Burlz10
Copy link
Author

Burlz10 commented Oct 4, 2023

Also @davidgfnet or @andymcca while I can confirm that the summon animations for Golden Sun are fixed (thanks for this) with this new update. I’ve encountered a new issue. ALL RTC based games/events do not seem to work anymore. I tried running Pokemon unbound after converting my .SAV to .SRM and it produced an RTC error. In addition, I tried starting a new Pokemon Emerald save and after saving and restarting it produced the same RTC error. Is there an easy fix for this I’m missing or is this a bigger issue that I should file a ticket for?

@DerinQyu or @Darkstalix. Could one of you guys attempt to run a old or new Pokémon save and confirm I have this correct?

@DerinQyu
Copy link

DerinQyu commented Oct 4, 2023

Also @davidgfnet or @andymcca while I can confirm that the summon animations for Golden Sun are fixed (thanks for this) with this new update. I’ve encountered a new issue. ALL RTC based games/events do not seem to work anymore. I tried running Pokemon unbound after converting my .SAV to .SRM and it produced an RTC error. In addition, I tried starting a new Pokemon Emerald save and after saving and restarting it produced the same RTC error. Is there an easy fix for this I’m missing or is this a bigger issue that I should file a ticket for?

@DerinQyu or @Darkstalix. Could one of you guys attempt to run a old or new Pokémon save and confirm I have this correct?

Please open a new issue and post a log file. On my ps vita pokemon gaia save files are working fine no rtc error.

@Darkstalix
Copy link

I just tried Metroid zero mission and Fusion, pokemon fire red, all 3 castlevanias, FFV, and so far all worked. I tried a new Pokemon emerald save game. Saved, closed and restarted emulation and i didnt get any rtc error and loaded up ok. Pokemon Unbound is a hack but i didnt have an old save to try to load to get the error. Tho i tried it on the latest 2.1.1.1 with a new savegame and i didnt get the error. Make sure in the core options while IN game set the RTC to ON cause if you read the description of the setting it says that you might have to set it to on for hacks and non commercial files

@Burlz10
Copy link
Author

Burlz10 commented Oct 4, 2023

Apologies must be an issue with me then. I’ll try setting RTC from auto to on and play around with the Pokémon unbound and Pokémon emerald saves and see what happens.

Edit: Turning the RTC from auto to on wasn’t working. But I tried deleting the auto save and launching from the .SRM instead of resetting from the quick menu and there was no error anymore. Resetting from the auto save doesn’t prompt the error anymore. Not sure why it worked but hey it’s all good now. No ticket required. Thank you though.

@andymcca
Copy link

andymcca commented Oct 4, 2023

@Burlz10 good to hear the issue is resolved. To clarify, there used to be a a core option to choose between gpsp save method (sav) or libretro save (srm) method. Both files are the same but with different extensions (although RetroArch does allow you to compress save files too). The gpsp option was removed when implementing save overrides (for games that employ copy protection such as the Dragonball Z games) for the sake of simplicity.

With all the recent changes to the core, Save States have had to be changed also, meaning States from earlier versions of the core won't work now. So yes, please rely on actual game save files instead when upgrading to the latest version.

@Darkstalix
Copy link

Since i got a bit confused from the post and English is not my first language. Does that mean that we will use srm from now on gpsp core?

@andymcca
Copy link

andymcca commented Oct 4, 2023

Yes srm only from now on (no more sav files)

@mitchchn
Copy link

The issue is not Android-specific, reproed in gpSP 1.15 on Mac, Miyoo Mini Plus, and RG35XX-H. It also shows up in more places in Golden Sun, for example with these two NPCs in Xian:

Screenshot 2024-05-19 at 1 34 19 PM Screenshot 2024-05-19 at 1 35 05 PM

Here's a save state in Xian. Walk up to the waterfall and wait for the girl to pass in front of it to see the second example.

Golden Sun (USA, Europe).state.zip

andymcca added a commit to andymcca/lr-gpsp-amcc that referenced this issue Aug 24, 2024
This is my attempt to support OAM Order Hijacking.  The cause and effect of this is outlined in issue libretro#227.

The issue that I see for gpsp in addressing this issue is that we store the Layer Priority at the Object/BG levels only.  In order to correctly deal with OAM Order Hijacking in the same way as the hardware, we need to be able to check priority at a pixel-level and act accordingly.

My solution here is - 

a) to check for OAM Order Hijacking at a row-level when we are ordering our objects (in order_obj), and mark each applicable row in the same way we do for rows that contain transparent objects.
b) when we come to render a scanline marked as hijacked, generate an OBJ-only scanline buffer first by rendering all available OBJ layers to this buffer in u16/INDXCOLOR mode, which gives us the most flexibility later on
c) when rendering 8-bit per pixel OBJ tiles, we check 4 pixels at a time for transparency (tilepix).  In the current code, if all 4 pixels are transparent (i.e. 0), we move on to the next 4 pixels to complete the tile.  But in my PR, we instead check each pixel to see if we have a non-zero value in the OBJ scanline buffer we created.  If we do, we render the value to dest_ptr using the selected render mode.

I have tried to build in some optimisations here such as only creating the OBJ scanline buffer on scanlines affected by a hijack and only comparing to the scanline buffer when we have 4 consecutive transparent pixels as described above.  The latter is sufficient to address the issues identified in Golden Sun as far as I can see, but complete/correct emulation would be to compare against EVERY transparent pixel.  

Further ToDos:
- 4bpp support
- Affine/Mosaic Objects support

So this PR is still in draft at the moment for more discussion/testing before committing.
@andymcca
Copy link

andymcca commented Aug 24, 2024

@mitchchn @DerinQyu @Darkstalix @Burlz10

Update on this issue - I've put together a draft PR which fixes it. Still needs further discussion and refining, but some progress at least!

@Darkstalix EDIT - don't worry, I found a similar save and worked with that. Now fixed in my draft PR! Before -

Screen.Recording.2024-08-24.172931.mp4

@andymcca
Copy link

After -

Screen.Recording.2024-08-24.173022.mp4

@Darkstalix
Copy link

Darkstalix commented Aug 25, 2024

NICE!!! Finally you managed this you beasts :D!!!
When is the new build coming out for testing?

thanks so much

@andymcca
Copy link

@Darkstalix no problem!! David running a full test against all Roms first to make sure it's ok, plus we have to refine the code a bit more before committing, so it may be a while. Will update here when it happens tho 👍

@Apaczer
Copy link

Apaczer commented Aug 25, 2024

@andymcca, seems fine on armv5 - no perf. drawback IMO (at least on two games I tested). Tried also savestate from prv msges, and no objects/background ordering issue 🎺 🎺 .

@andymcca
Copy link

@Apaczer thanks for the test and report!! That gives me more confidence about it, plus I already know theres a bit more room for further optimisation too. 👍👍👍

@Burlz10
Copy link
Author

Burlz10 commented Aug 26, 2024

No way! That’s awesome. Thanks for figuring this out guys. Looking forward to being able to play golden sun without this glitch.

@andymcca
Copy link

No problem @Burlz10 ! If you're comfortable building from source then you can try it out now from my PR but it needs more work/testing/refinement before it's going to be committed I think. Will post further updates on your issue here tho!

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

No branches or pull requests

7 participants