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

Digimon World HLE bios issue #46

Open
gameblabla opened this issue Feb 10, 2019 · 10 comments
Open

Digimon World HLE bios issue #46

gameblabla opened this issue Feb 10, 2019 · 10 comments

Comments

@gameblabla
Copy link
Contributor

gameblabla commented Feb 10, 2019

Digimon World had two issues with the HLE implementation
The first one occurs when it uses the strcat by attaching ;1 to a string. Disabling the function lead to the game to boot, which led me to nocash's psx documentation and it turns out the function is not correctly implemented. It also needs psxRegs.CP0.n.Status |= 0x401; for it to work properly and go past the lock up, whatever this does. (related to IRQ ?)

The second one is input related : it would not detect button presses.
After much frustration trying to debug it, it turns out to be yet another missing check.
Doc says that Bx015 (OutdatedPadInitAndStart) will only work if type is 0x20000000 or 0x20000001.
If it's not, then it will refuse to do anything. Implementing this behaviour also fixed input in digimon.

You can see my fixes to those here
gameblabla/pcsx4all@79411d3

Unfortunately, while the game is playable, it still has some issues.
The other one is that sound effects will not playback (but other sounds will work ok). The other, more annoying issue is related to memory cards. If you start a game in Digimon World, save it, exit the emulator and try to continue the game from the game itself, it will fail to detect the saves in there.
And when you attempt to start a new game over the one you were using, it will refuse to start the game. I believe that the game will not create a new game and overwrite it over an existing one.
But it still shows as if no saves exists. Card_info function is used for checking saves (apparently).

So yeah, while i did allow the game to bootup and be playable, there are still other issues. I would be happy if anyone could fix these. (i will also let pcsx-rearmed about it but notaz isn't too active so i doubt)

@dmitrysmagin
Copy link
Owner

Thanks for the fixes, I'll incorporate them asap, though, I'm not in active development now.

@gameblabla
Copy link
Contributor Author

gameblabla commented Feb 11, 2019

Well i kinda figured that you're not working actively right now. Just thought i would let you guys (senquack and you) know, especially since no one cares.
Btw, i also managed to fix the memory card issues on Digimon World and i merged a fix from someone else on a board for the LEGO Racers save issue.
I also improved my HLE implementation to make sure it complies with nocash's documentation
See my huge patch here
gameblabla/pcsx4all@dcabe2e

If you only care about fixing the memory card issue, then check out buopen and card_info.
Now the only issue left are the missing sound effects in menu and missing menu ingame. But i think the SPU plugin is partially to be blamed here.

@senquack
Copy link
Contributor

I'd already caught the one PAD_init type-argument checking bug, while checking mem alignment bugs elsewhere in the project (Need for Speed 2,3 need this fix). Thanks for notifying of the other HLE issues. I am currently doing unrelated work in HLE regarding removal of unneeded ResetIoCycle() calls. I am working on other things right now, but will revisit HLE in the future. For now, just advise users to use a real BIOS, HLE has many issues as I'm sure you're aware.

Issue I see with some of your HLE patches:

  • I tested with Digimon World SLUS01032, and found that neither of the fixes listed in commit gameblabla/pcsx4all@79411d3 were actually needed by that game. Furthermore, you randomly added a enabling of CPU CP0 HW interrupts in a 'strcat' function of all places. This somehow did allow Digimon World to boot, but it was merely by chance and should not be accepted as a patch.
  • I checked out your pcsx4all fork which included more HLE patches (some of which are huge and should be split, and also have newline formatting issues). There, I removed the strange line mentioned above that had bizzarrely added setting of CP0 Cause register to 'strcat'. Digimon World still worked. This means that (thankfully) that hacky weirdness can go.
  • It's great that someone is working on HLE BIOS, but I am indeed still working on the project and it's going to be a nightmare to keep this coordinated, especially if patches aren't being made in a rigorous clean manner so they can be accepted upstream.

@senquack
Copy link
Contributor

I'd also like to add that some of these patches from Steward are useful (4-pixel, 16-pixel sprite rendering optimization is good, I know this because I've verified something similar myself). However, part of his solution involves this:

gameblabla/pcsx4all@7a849c6

If someone is going to go through the trouble of adding an ASM memcpy, it better be better than what C could generate on its own! This code would take (assuming no L1 misses) a minimum of 10 cycles each loop. It could be cut down to 7 cycles with just 30 seconds of effort, and C likely would have done that for you. Furthermore, your libc likely already includes a MIPS ASM memcpy that does much better than even 7 cycles per loop, massively unrolling the entire copy and reducing L1 miss effects.

Now, this

@senquack
Copy link
Contributor

.. (left from other message)
This could be fixed in the future, but for right now I need to finish other things, personally. I do think these HLE fixes are very useful for future inclusion, in a cleaned-up state.

@gameblabla
Copy link
Contributor Author

Alright so i managed to narrow down the boot issue in Digimon World to two commits.

This one, which fixes strcat and add some checks to it
gameblabla@bd3fe0f
And this other one, which is very important and properly implements the BIOS behaviour of EnterCritical. This allows games like Medievil 2 to go in-game and for Digimon World to boot.
gameblabla@2a416ff

Now, the game will boot but input will not work at all and this can be fixed with this (i explain above what it does)
gameblabla@2427ad2

But then, the game will refuse to start a game and behave very strangely with memory cards. You can start a new game but if you reboot and try to continue your game, well you can't.
It requires the following fixes from upstream to fix this
gameblabla@1da37f8
gameblabla@cbd87fe
And possibly this one too
gameblabla@f6cff7c

After that, the game will now behave properly with saves. Unfortunately, the issues with missing sound effects & music still remain but this does allow the game to be played properly now.

@dmitrysmagin
Copy link
Owner

dmitrysmagin commented Feb 13, 2019

Are all your commits in hle_improvements branch are essential? Or some of them are random?
I'd recommend you not to multiply repos, but create several new branches like hle2, hle3 etc and place your reworked commits there. For now they seem to be random with much errors in codestyle (curly braces after 'if'), trailing spaces etc. Some of them could be squashed together.

Don't get me wrong, I appreciate your work, but for now it seems easier for me to squash your changes into one mega patch, fix codestyle and commit as-is.

@gameblabla
Copy link
Contributor Author

gameblabla commented Feb 13, 2019

Not of them are not essential, they are general improvements to the HLE.
Senquack told me that i should separate them this way (like he does so for his MIPS recompiler). He did take issue with some of my patches (mostly SystemError & ReturnZero) because he thought these would not be useful.
As for the the trailing spaces, well those kinda pisses me off, i seriously need to change to another text editor. (does not help that github is not making them obvious...)
Should i just merge these few patches for Digimon World and merge the rest later in their own separate branches ?

@dmitrysmagin
Copy link
Owner

It's better to squash Digimon patches into one with a descriptive commit message. Btw, are you aware of 'git rebase -i' ? :) It's an easy way to work with existing commits and move them around, merge or delete. Other improvements should stay separate, I suppose. It's ok to accept them if they don't break existing games.

@gameblabla
Copy link
Contributor Author

Alright, i did as you said and only did a pull request for allowing the game to boot here :
#48

It doesn't fix the memory card issues yet, i will issue another pr for that but the game still boots and can be played with the pr. (it also properly fixes other unrelated games)

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

3 participants