hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
11.44k stars 2.19k forks source link

ULJS00218 - Hitman Reborn Battle Arena 2 - Player 2 side broken/reversed/broken kick animation #12900

Closed somepunkid closed 3 years ago

somepunkid commented 4 years ago

What happens?

In-Game/Fights, particle effects and physics only applying correctly on Player 1 side during fights. On Player 2 side, properties operate as if they are being performed from P1 side. This leads to reversed effects and physics being applied to everything from P2 side. This effectively makes the game unstable/unplayable, since only one player side functions correctly.

What should happen?

Here is the direct comparison with the same variables on both hardware and emulation. https://youtu.be/q1YjrLXTq7c

What hardware, operating system, and PPSSPP version? On desktop, GPU matters for graphical issues:

This was most recently tested with the UMD directly, to ensure there wasn't an issue with the source file being tested.

Win10 / Android (unknown config for android, was tested on two devices in the past and not working) PPSSPP v1.9.3 Direct3D11/9, OpenGL, Vulkan Nvidia 2080 Max-Q and Nvidia 970m tested with current build. Many other configs have been tested with previous builds over multiple years.

unknownbrackets commented 4 years ago

Ah okay, I left it using the incorrect/low precision sin/cos but I get it, harder to verify you've reproduced that way.

PPSSPPWindows64-sincos-lowprecise.zip

Here's a new one. It keeps the incorrect sin/cos values, but only for uneven sin/cos values.

If the issue happens here, the log should tell me a lot. If it's not some sign issue, this will be in direct conflict with previous CPU tests and Final Fantasy 3's behavior, so would imply it's actually somewhere else and this just "papers over it" somehow.

If the issue doesn't happen here, it means it's a precision issue. Our sin/cos is probably too precise for some values.

-[Unknown]

somepunkid commented 4 years ago

Ah okay, I left it using the incorrect/low precision sin/cos but I get it, harder to verify you've reproduced that way.

PPSSPPWindows64-sincos-lowprecise.zip

Here's a new one. It keeps the incorrect sin/cos values, but only for uneven sin/cos values.

If the issue happens here, the log should tell me a lot. If it's not some sign issue, this will be in direct conflict with previous CPU tests and Final Fantasy 3's behavior, so would imply it's actually somewhere else and this just "papers over it" somehow.

If the issue doesn't happen here, it means it's a precision issue. Our sin/cos is probably too precise for some values.

-[Unknown]

Same thing, now the console isn't logging much. The game is running correctly with this executable as well, from what I can tell. If I switch back to the executable that came with this package(ppsspp-v1.9.3-908-g192164c5f), the game runs incorrectly again.

I cannot reproduce the in-game behaviors with PPSSPPWindows64-sincos-lowprecise

somepunkid commented 4 years ago

So, perhaps the precision issue then. Interesting stuff.

hrydgard commented 4 years ago

Could it be some other special values, maybe sin/cos of inf/-inf ? Just guessing here, didn't see that in the posted log.

somepunkid commented 4 years ago

I was able to find a difference / issue on the working builds with the game. This was tested on PPSSPP 0.9.8 and ppsspp-v1.9.3-908-g192164c5f with the above sincos-lowprecise exe and PPSSPPWindows64-revert-sincos exe, (Interpreter) versus hardware (PSP/PSTV tested)

https://youtu.be/EPtkQhr0Bps

I made a log while testing this inconsistency (PPSSPPWindows64-revert-sincos exe / build ppsspp-v1.9.3-908-g192164c5f ), but I don't know if it will have much more information. Possibly just a separate, unrelated bug.

https://gist.github.com/somepunkid/aa1dc0ea4bff1bcb257a9c13b913d2c5

EDIT: This above can be ignored if it's not related to the main issue. I finished my frame counting for the game and only found this single gameplay inconsistency with the modified Core files/ exe [Unknown] provided above. The game works perfectly otherwise with just that.

somepunkid commented 4 years ago

Greetings!

I've been testing the most recent build in from Orphis.

v1.10.3-686-g68735b4e5

I'd like to see if I could test this current build with the modified exe/variables from above created by @unknownbrackets Any way I could have this built again (the changes in PPSSPPWindows64-revert-sincos.zip) for this recent revisions assets (v1.10.3-686-g68735b4e5) and linked? I wanted to see if I could reproduce a couple bugs with the most recent changes.

https://github.com/hrydgard/ppsspp/issues/12900#issuecomment-632269117

If this is possible, thanks so much!

hrydgard commented 4 years ago

Is it impossible for you to set up so you can build yourself? It's actually not that hard.

If the build instructions below are not enough, feel free to contact me on PPSSPP's Discord server.

https://github.com/hrydgard/ppsspp/wiki/Build-instructions

somepunkid commented 4 years ago

Is it impossible for you to set up so you can build yourself? It's actually not that hard.

If the build instructions below are not enough, feel free to contact me on PPSSPP's Discord server.

https://github.com/hrydgard/ppsspp/wiki/Build-instructions

Hey there, hrydgard. I did try to make the changes manually and compile it at one point, but ran into some difficulties/errors and was unable to troubleshoot the cause, is all. I spent quite a bit of time with that guide you linked! I'll see if I missed anything...

hrydgard commented 4 years ago

Please give it another shot - it will be very useful to have you experiment directly so we can pinpoint more exactly which change does the trick. Like I said, happy to help you get it set up.

somepunkid commented 4 years ago

I was able to complete a build for testing in v1.10.3-686-g68735b4e5 with the reverted changes from #6329 (https://github.com/hrydgard/ppsspp/issues/12900#issuecomment-632264965). The game still works 99% correctly on this revision of PPSSPP.

The only issue/inconsistency left I could find that differs from hardware-based play still exists even with this/reverting #6329.
I'm not sure which variables would be needed to tweak for further testing VS the native hardware for this games performance, as this same issue existed on all the working builds of PPSSPP using this game prior. Tested all the way back to ppsspp-0.7.5-win, up to 0.9.8, latest being ppsspp-v0.9.8-1178-g036cde776-windows-amd64. Couldn't get the character/characters move to work on any earlier version. (Documented here https://github.com/hrydgard/ppsspp/issues/12900#issuecomment-632629010)

This may be a separate issue. The reason I'm listing it is because I'm unable to tell if these variables also impact this secondary issue. That being said, not a massive deal, this is an inaccuracy/impacts one character in game. (although, it is a balancing difference). Reverting these changes doesn't do it either, so it's not likely.

Other than this, the game works 99% correctly with the reverts of #6329 with the most recent build from what I can see, so that's something. In addition, I'm not sure how much the changes reverted in #6329 being tested here will impact other games, so unfortunately, I'm unsure what would be needed from here...

hrydgard commented 4 years ago

I would be really interested if you could revert the #6329 changes one by one, and try to figure out exactly which part is the one that makes a difference here... Then let's take it from there.

somepunkid commented 4 years ago

I have isolated the first issue to two changes listed in

https://github.com/hrydgard/ppsspp/compare/master...unknownbrackets:sincos-log

Under file: Core/MIPS/MIPSVFPUUtils.h

image

Specifically reverting just lines lines 41 and and 54 of this file, the game stops reversing/has normal behavior/physics and effects don't reverse. It should be noted that reverting just one or the other does not resolve the behavior, only both. I tested each instance. This is to isolate the bug that was originally submitted. Tested with build ppsspp-v1.10.3-698-gccea07ab9-windows-amd64.

I'm unsure if any of the other modifications of Core/MIPS/MIPSVFPUUtils.h are necessary/have an effect. Just modifying these two led to visibly normal behavior and play. I've yet to find another issue, besides the secondary listed issue which persists, so this may be related to something else.

somepunkid commented 4 years ago

I've done some extended tests with this build now, and it is still running correctly (interactions, physics, distances all stable) with just these two changes, so it must have something to do with variables impacted by these.

somepunkid commented 4 years ago

As for the other thing, that means this issue is separate https://github.com/hrydgard/ppsspp/issues/12900#issuecomment-632629010

Is there a way to isolate what resource is being used during this exact moment/an exact moment in-game? Perhaps there a specific log that needs to be utilized? Essentially, this entire animation/properties both change. Whats odd is that it still only seems to be for this character, I haven't found another instance still.

hrydgard commented 4 years ago

Good work isolating it so well. I think we'll need to do some more extensive hardware tests - but also se can switch to the old calculation for this game in the meantime, similarly to the dot product precision thing we do for Tekken 6....

somepunkid commented 4 years ago

Good work isolating it so well. I think we'll need to do some more extensive hardware tests - but also se can switch to the old calculation for this game in the meantime, similarly to the dot product precision thing we do for Tekken 6....

Of course, glad it could be found. That makes sense, switching it back does totally stabilize the game. It's possible those future tests will have results/calculations resolving both issues. For now, we can mostly fix it with the reverts :)

somepunkid commented 4 years ago

Good work isolating it so well. I think we'll need to do some more extensive hardware tests - but also se can switch to the old calculation for this game in the meantime, similarly to the dot product precision thing we do for Tekken 6....

Hey there, hrydgard. I do have one quick question. For us to switch to the old calculations for this game in the meantime in future builds, is there anything else left I should check/try in a separate build besides isolating it to these lines? Thanks!

somepunkid commented 4 years ago

I've continued to create builds with these two changes as the emu has updated and tested them. It is stable, even in MP. I'd def say a compat fix would be helpful for now so it can update automatically to future revisions.

hrydgard commented 4 years ago

Great!

Sorry I got distracted by some other stuff but I'll get a compatibility workaround in soon.

Then I plan to do additional reverse engineering of these instructions, but not sure when I'll get to that...

somepunkid commented 4 years ago

Great!

Sorry I got distracted by some other stuff but I'll get a compatibility workaround in soon.

Then I plan to do additional reverse engineering of these instructions, but not sure when I'll get to that...

All good! Very cool, thanks! The compatibility workaround will make it easier to test variables for any other issues in new builds, for this game at least, until the whenever hardware tests are looked into.

hrydgard commented 4 years ago

Made a PR, #13523 - can you test it?

somepunkid commented 4 years ago

Happy to. Just need to wrap my head around importing this PR with Git Bash. I'm still somewhat new :> EDIT: got it imported, testing now

somepunkid commented 4 years ago

EDIT: Apologies, I hadn't imported the compat.ini to the build I was testing.

So, this changed a number of things. Text in the game broke, many effects not appearing correctly/at all, just using the stock settings in v1.10.3-876-gcf669e6a2

for Core/MIPS/MIPSVFPUUtils.h https://github.com/hrydgard/ppsspp/compare/master...unknownbrackets:sincos-log the only two that made an almost perfect difference for this game were just reverting 41 and 54. When I did my local tests, that's all I changed! specifically back to - float checkAngle = angle - floorf(angle 0.25f) 4.f;

somepunkid commented 4 years ago

I have created a video demonstrating the differences between the latest build with and without the compat changes, as well as the build I'm using with just the two lines changed.

https://youtu.be/CrgD4ImWUJ8

The left instance is from an older version, with only two lines of code modified (ppsspp-v1.10.3-698-gccea07ab9-windows-amd64) It is the one I currently play, as it is totally stable minus one animation.

The upper right is the latest orphis build without the #13523 changes (v1.10.3-876-gcf669e6a2) Notice that the graphics/text remain the same, but the physics/inputs are reversed (because the sin/cos calc changes aren't applied)

The bottom right is the same latest orphis build with the compat.ini changes. Notice that graphical elements during strikes are different, text is missing etc. Functionally, it is correct from what I can tell, but with many more inaccuracies.

somepunkid commented 4 years ago

What I'm proposing is that, if possible, a workaround for this game for now would be most accurate with just these 2 line changes applied for compat, and possibly nothing else (besides the flags for it). I've ran multiple tests with the hardware present and it's damn near the same, minus that one kick animation at 1:40 that just sinks straight down (it's different on hardware, and has never been emulated correctly in PPSSPP or JCPSP. I am STILL unsure what the cause of this is, https://youtu.be/EPtkQhr0Bps)

hrydgard commented 4 years ago

Thing is, I have to duplicate those functions so we don't get a speed loss in the JIT (don't want to add additional checks at runtime). My "old" functions should function identically to your ones with one line reverted, unless I've made a mistake (which I may very well have!).

But I think I do have to call them in the interpreter as well, it doesn't work as I did to only do it in the JIT since it sometimes falls back on the interpreter. I'm gonna fix it up.

somepunkid commented 4 years ago

Does this also apply for a compat fix with this one instance? If so, does it make a massive difference for this game? It seems to run perfectly on everything I throw at it (even old hardware)

Why would there be so many visual differences?

somepunkid commented 4 years ago

I suppose something else may have changed, for half of the game text to suddenly also be missing.

However, even in older revisions, the missing text was another issue this specific game had. It may have been another one of these changed variables.

hrydgard commented 4 years ago

When the game relies on weird edge cases in CPU emulation, and we don't handle them correctly, bizarre things can indeed happen.

Regarding the speed thing, it's not that I think it'll slow down this game, I just don't want the fix for this to have an impact on other games. Anyway, I've updated the pull request with additional checks in interpreter mode.

I'm also going to try a different idea, stay tuned - I'll submit that as a separate PR for you to try.

hrydgard commented 4 years ago

Please try both the updated original PR (might need to do git --reset hard to it to avoid merge conflicts), and this new one: https://github.com/hrydgard/ppsspp/pull/13526

and let me know the results.

somepunkid commented 4 years ago

Ok, #13526 actually FIXES Byakurans kick animation, but still has the reversed physics that have been reported. Well, this is the first time I've ever seen this animation work correctly.

Testing #13523 now.

ALSO: I was implying we could apply a compat fix to just this game and not impact any others, somehow. Not sure if that was possible, thats all :>

I'll try 13523 again

somepunkid commented 4 years ago

Ok, the physics are corrected in 13523, but the games hit effects and text are still gone. The kick animation is broken again. Basically no noticeable change from when this was first tested.

somepunkid commented 4 years ago

Hit effects, text, and the broken animation are ALL fixed in #13526

The ONLY issue is the base issue (the reversed physics)

somepunkid commented 4 years ago

Logging these differences: https://youtu.be/hpVN07wjwWc?t=43

(#13526 left, 13523 right)

Should be the correct spot in the video for comparison/performance of these two, just for reference in case. The kick animation referenced can be seen on left with all text/effects and directly compared to the right, but the left side has the input reversing/broken P2 physics still unfortunately.

hrydgard commented 4 years ago

I made another commit to the #13526 branch, turning off the special direction checks - I'm thinking we can get away without now that we're in double precision. Try it!

somepunkid commented 4 years ago

It works minus the animation! Effects and text are present, and no reversing.

The only thing broken is that kick animation in the comparison video with this build. At least now I know that this animation is also related to a CPU emu issue...

hrydgard commented 4 years ago

Aha! I made one more change, forgot to turn off the checks in vfpu_sincos. Hope that works.

This game has to be doing some really bonkers math for this to be an issue...

somepunkid commented 4 years ago

Tested #13526 again, same deal with the kick animation. Everything else still seems fine.

somepunkid commented 4 years ago

Aha! I made one more change, forgot to turn off the checks in vfpu_sincos. Hope that works.

This game has to be doing some really bonkers math for this to be an issue...

It must be some oddball math problem!

I edited Core/MIPS/MIPSVFPUUtils.h /PR#13526 earlier (now line 63) to the old calculation (the really old one that I mentioned swapping originally) to see what I could test/break. This was while the kick was working, but the physics being broken. It lead to the physics/reversing being resolved, but the kick broke again.

I'm only outlining this to see if it helps isolate anything.

hrydgard commented 4 years ago

Okay, well, at least it seems to be an improvement and maybe we can live with the broken kick until someone goes really deep on these functions. So this might be the right way to go for now, but need to test stuff that was fixed by #6329, such as #2921.

somepunkid commented 4 years ago

Yeah, it's entirely functional otherwise, from what I can tell. Doing a fresh test to ensure.

EDIT: Looks good, not seeing any other major issues.

somepunkid commented 4 years ago

Yup, just did MP tests with two instance, all green.

I'd love to get that kick fixed as well, if I knew what variable to mess with but...ehhh. Sounds like this comes down to the right math going one way or the other. Why do you think it worked earlier in testing #13526? I'm sure you have an idea what variable this might be...

somepunkid commented 4 years ago

I read the notes for #13526, and well, as I guessed earlier, that one is likely a fix for later. It was cool to verify what was causing all of it though. I can stop messing with the applications settings trying to fix it :>

hrydgard commented 4 years ago

I've merged #13526 to master, but I'll leave this open since the kick animation is still broken.

somepunkid commented 4 years ago

Ok, very cool. I would happily test any additional changes for this one as time goes. I'll probably check it out on Android today.

somepunkid commented 4 years ago

Update: Broken physics/reversed issue is on Android, but working on Windows lol.

EDIT: Tried in Interpreter as well, was still broken on Android. If you have any suggestions for testing it here as well, pass them my way!

EDIT AGAIN: Tried build v1.10.3-892-g3fe1e35e3

Suddenly working. I'll keep messing with it to see if it stops. Had a crash in the pause menu once...but it's been fine since. I'll see!

hrydgard commented 4 years ago

OK hehe, yeah we don't do anything differently on Android here, but since it's different CPU architecture, and the slightest floating point differences seem to trip this game up, I would have believed it...

somepunkid commented 4 years ago

OK hehe, yeah we don't do anything differently on Android here, but since it's different CPU architecture, and the slightest floating point differences seem to trip this game up, I would have believed it...

Understandable :>

I've continued to play on Android, couple random crashes but has otherwise been mostly stable. Kick doesn't work on Android either, of course

somepunkid commented 4 years ago

I did not mean to close this. Damn mobile buttons

somepunkid commented 4 years ago

Been getting a lot of crashes in game on Android, seemingly random during matches. Not sure if this would be related to this same issue. Crashes the whole emulator. Don't remember it doing so before...but it also had the messed up physics before so this is much more playable lol.

Haven't had this happen on Windows yet.