ps2homebrew / Open-PS2-Loader

Game and app loader for Sony PlayStation 2
https://ps2homebrew.github.io/Open-PS2-Loader/
Academic Free License v3.0
2.05k stars 262 forks source link

Merging GSM and PS2RD into OPL permanently - do we want it? #120

Closed sp193 closed 5 years ago

sp193 commented 6 years ago

Given that GSM and PS2RD can be disabled properly, do we not want to merge them into OPL permanently? There will be less choices for the user, which gives increased simplicity.

I know we previously had some concerns about why enabling/disabling some features seemed to affect games. But after Fatal Frame was fixed, I have this feeling the reason for the varying functionality (even GSM or PS2RD is not enabled for the game) must be related.

For the uninitiated: Fatal Frame has started working after a recent patch, which wipes all (close to what Sony does) registers so that the boot conditions will be always the same. I noted that the only visible difference between running the game with PS2RD built in or not, was the register content. There was also this bug whereby OPL would corrupt memory at 0x02000000. I actually dumped all memory to see if there are differences in the memory meant for the game, but no. There were differences, but they were in the stack area and the EE kernel.

If there are games that are still affected by whether we include these features (when disabled during runtime) or not, then I am sure it has got to do with a problem within the game itself. Even if we do sweep the issue under the rug by disabling these features, it might occur again whenever the EE core grows in size. So by this logic, I think it is okay to have these features built in.

As for VMC: I was told long ago that it can result in worse performance in some games, but I never figured out why. That is why I am not including VMC in this suggestion, although I guess we could if we reach a general consensus that it is okay. Technically, the VMC module is also not loaded when that function is disabled. So the overhead might have just been related to the additional WaitSema() & SignalSema() calls that the VMC builds have. That was also a long time ago, before a lot of fixes and enhancements were made.

Jay-Jay-OPL commented 6 years ago

Are you referring to that we won't need to enable those modules during compiling?

If yes, then I vote yes, I think we should make them a permanent feature in OPL. I also haven't come across a situation where those two modules need to be excluded when trying to play a particular game.

As a popular wishlist, many users have requested that these features get enabled for the childproof version:

I guess they want to be able to use those features for their children. Maybe not able to adjust those features, but more like it should respect what has already been saved in the game's CFG file with a non-childproof version. The same way, I believe that childproof users can still use a VMC.

This way if their kids load a PAL game on a NTSC TV, they won't worry that the childproof version won't load it. Since it will require GSM to be enabled and the NTSC option to be selected.

Or if they want their kids to also play the games with the cheats they've already added to the CHT folder.

TnA-Plastic commented 6 years ago

While I am all for 'cutting down' on the compile-options AND also for implementing both features permanently, I would really like to see a 0.9.4-release before and thus a test of the crowd out there... If we encounter games, which still have incompatibility with one but not another build, then that might be a useful feature to keep. ;)

rickgaiser commented 6 years ago

I'm all for integrating all compile-time options that can also be enabled/disabled at run-time. GSM and PS2RD should have no effect on games when they are compiled in, but disabled.

As for VMC, I alsways use builds with VMC compiled in and never have problems when not using VMC. So as far as I'm concerned we can merge VMC into OPL also.

I would really like to see a 0.9.4-release

Me too, but I'm not sure if we will ever reach consensus about this. So I think we should proceed with new features/improvements, and not wait for something that might never happen. The crowd can still test all versions we make.

TnA-Plastic commented 6 years ago

There EVER is a new feature 'to be axded'! I think it would be good, if a 'stable' tagged version could be released, because implementation-failures could be immediately being reported by a MUCH bigger crowd! ;)

sp193 commented 6 years ago

Thank you all for your replies!

I'm all for a "v0.9.4", as long as it has some improvement in game compatibility, so that users of its core business functions will find it worthwhile. Well, now there is; it's now compatible with at least 7 more games, compared to v0.9.3.

For now, I have merged in CHEAT, GSM, VMC and CHILDPROOF: https://github.com/sp193/Open-PS2-Loader/tree/gsm-cheat-childproof-merge Test build: https://www.sendspace.com/file/t4krh4

Childproof is no longer a compile-time option, replaced in favour of having a parental lock option. The master password is 989765, in case there is a need to clear the parental lock. To use the password, perform any restricted action (i.e. change settings) and enter this master password. Upon entering the master password, the parental lock will be disabled and the settings will be saved. When the parental lock is enabled, any settings that were set before will still be binding. For example, if you have GSM 480P mode enabled for a game and your son/daughter starts that game, GSM will still function.

TnA-Plastic commented 6 years ago

I have some concerns about the BDM-Implementation, even tho' it is well done! I still believe that alot bugs will occur in the first revisions.

I hope for @rickgaiser 's new commits/test-builds (regarding 'HiRes') to be merged into the main-repo and 'gskit' and that we have a '0.9.4' ('kind of'...'stable'), before too much new stuff gets merged into it...

A lot stuff like the CLs are based on stable versions and all reimplementations affect various stuff alot, so a release would be neat, indeed! ;)

sp193 commented 6 years ago

I've just gone ahead to merge VMC. Please use the new link.

Jay-Jay-OPL commented 6 years ago

Okay, I just tested your most recent update (with VMC merged). The childproof works awesome. VMC, CHEATS, GSM, are still loading/working when we launch the PS2 Game in Childproof mode.

Plus I like how you implemented how it works now, that if we have the Parental Lock Settings enabled, it will ask for our custom password each time we want to enter a settings page. -- UPDATE: I also like it that it's based per session, once we enter the password to enter any settings page, it will stay open until I guess we relaunch OPL.

I also like that there is a master password in case the end-user forgets what he set as his own custom password. I doubt toddlers will be able to Google Search for it. So it makes sense to have a master password.

Good job! :)

ElPatas1 commented 5 years ago

SP193, are you sure that the VMC can't cause problems to the games for to be integrated?

Also doctorxyz said do not use IGS together with VMC, and if VMC is integrated then there is no way for use both of them separately.

Best regards.

sp193 commented 5 years ago

@Jay-Jay-OPL: thanks for your feedback.

@ElPatas1: my suggestion is to merge VMC with the rest of OPL, but it will still not be enabled by default. The only difference now (when VMC is disabled), is an extra export table, a semaphore for sharing the device between OPL's CDVDMAN and MCEMU, and write support. Not really much more code. But other than those, it is the same as when VMC is not built with OPL. So you can see that in theory, there are not many reasons why it would break games or be incompatible with IGS, just by being built in...

But in case this really is not a really great idea, this is the period for people to complain here and to be heard.

Personally, I think merging these features can lead to 1 more benefit, other than giving users fewer (but better-featured) choices: there will be less "ifs" around the source code, which made system.c look confusing IMO.

ElPatas1 commented 5 years ago

I see, the parental look is disabled by default right?

What i noticed with the new string of parental look is that it messes up the order of the translation languages with the PADEMU words.

I can provide it to the people for test?

Best regards.

sp193 commented 5 years ago

Yes, the parental lock is disabled by default.

Regarding the PADEMU language strings: the problem is that they are only included when PADEMU is built at compile-time... so I couldn't tell what is the "normal" language file like. Do you know if translators are currently translating with the PADEMU lines included or not?

That can be standardized in another commit. If the PADEMU lines are usually included, then I will add the new lines after the PADEMU lines. Otherwise, they will be added before the PADEMU lines. In both cases, I shall make the PADEMU lines permanently part of the strings table to solve this problem.

Yes, please feel free to provide the ELF for other people to test.

ElPatas1 commented 5 years ago

Yes, we translated some languages files with all the PADEMU lines included.

Strings are first located at src/lang c, then there is an english template which includes all the strings from src/lang c, and from this template we translate to all the languages.

Best regards.

sp193 commented 5 years ago

I see, thanks. I have made the PADEMU lines part of the standard strings list, and all new lines will be added after those.

As for this topic of merging the stable features - related commits: https://github.com/ifcaro/Open-PS2-Loader/pull/128

sp193 commented 5 years ago

Branch has been merged. We now have VMC, GSM, PS2RD (Cheats) and the Childproof features merged in.

TnA-Plastic commented 5 years ago

As long as it does not cause issues, that's really great! This decreased the amount of possible builds quite a bit! Great work!

...and the parental lock is pretty cool and seems to be well implemented. :)

ElPatas1 commented 5 years ago

Hi sp193,

the PS2RD do not works well after the integration, an user reported that the games freezes when saving the settings after are launched. Freezes with the beta test version you provided ant with latest betas.

With the beta after the integration games don't freezes.

Concretely he tested first this game and cheats:

ID SLPS_25783 "The King of Fighters '98 Ultimate Match" Enable Code (Must Be On) 901105D8 0C04411E Alt. (M) F0100208 00119CCB P1 No Power 00680639 00000000 00680733 00000000 0068069D 00000000

Even without activating any cheat from the PS2RD before, the same thing happens, it hangs after launching any game from these latest beta versions.

Best regards.

Jay-Jay-OPL commented 5 years ago

@ElPatas1

I've tested a handful of games with PS2RD running and I've had no issues. Maybe the cheats he is using are bad? Tell him to try a different game and to pay close attention to the cheats he is using--could be wrong for the game.

Also what device is he using (USB, HDD, or SMB) and does he have VMC enabled?

Plus looking at the cheat codes above, this should be removed:

Alt. (M)
F0100208 00119CCB

It doesn't seem he converted these cheats correctly to RAW. Since PS2RD will not accept a mastercode (M) that starts with F0 -- so most likely scenario, he doesn't know what he is doing with these cheat codes.

Can you confirm his bug with some of your games that have cheat codes or not?

UPDATE: I currently disabled the cheats for a game that has cheats, and I was able to make it load. I will try other games to see if there is a bug somewhere...

Jay-Jay-OPL commented 5 years ago

UPDATE 2:

Okay I did several tests with the same game: SLUS_209.43.Heroes of the Pacific.iso via SMB

Testing with: Open PS2 Loader 0.9.3+.1166-Beta-e57ded9 w/IGS , PADEMU, and HIRES enabled.

  1. Game with no CHT file in the CHT folder and cheats disabled for the game -- it will load fine.
  2. Same Game with a CHT file in the CHT Folder and cheats enabled for the game -- it will load fine and the cheats work fine.
  3. Same game with CHT file in the CHT folder and cheats disabled for the game -- it will load fine.

The RAW cheats can be found here: http://www.ps2-home.com/forum/viewtopic.php?f=55&t=6125

Let me know what tests you've done to troubleshoot and find the issue...

TnA-Plastic commented 5 years ago

@Jay-Jay-OPL: Could you use a VMC and save, after the cheat got applied?

You are probably also right about the Mastercode, which I think needs to be a F-code, instead of a 9-code... BUT... Essentially this is a super easy fix... Changing the 9 to an F...

Jay-Jay-OPL commented 5 years ago

Yes, I was able to test the game with VMC enabled and also with just MC. And did not find an issue with that.

I think you got the 9 and F mixed up when it comes to mastercodes for PS2RD.

PS2rd only works with mastercodes that start with a 9 and not an F.

Plus in the cheats listed above he has a 9 mastercode then the other one that starts with an F. -- Plus I don't think it's that easy about just switching the first digit. I've never tried it, but never seen anyone suggest that to work. Hmm?? If it were that easy, there would be no need for the application "Mastercode Finder".

When it comes to game cheats is that if a cheat code isn't right, it can cause the game to freeze up or not run, and since he had that questionable F0 mastercode there, that could be the culprit or a clue he doesn't know what he is doing.

I can't test this game, since I don't have it, but the cheats listed don't look correct. That is why I suggested for him to ask that guy to look into that, to double-check if he actually knows what he is doing and yada yada yada...

Plus Japanese cheat codes are not easy to find, so maybe he is trying a NTSC or PAL cheat code for his Japanese game? So there could be many other possibilities.

sp193 commented 5 years ago

If it does not work for only 1 game, then you need to check if the cheats are valid. While you can see a lot of changes, I didn't actually change how the cheat engine worked.

Any issues with OPL freezing up when saving configuration after selecting a game (i.e. because he had the option for remebering the last game played enabled), might be related to existing issues within OPL or USBHDFSD. It has been going on for some time already, but I remember it was already like that... For some reason, this bug seems to be appearing and disappearing, even when unrelated changes are made.

On a side note, you may want to update your ps2-packer builds because I might have fixed the design flaw that caused bad files to be made. I couldn't really establish what effect the packed data has on the EE because the bit patterns there do not always match the documented cases for reserved instruction patterns that cause malfunctions. But I have not encountered a single bad file being generated, ever since I made the change... If it was really involving the related EE hardware restrictions, then the bit patterns might involve the documented problems: data may be destroyed, wrong instructions may be fetched, FPU may malfunction etc. Depending on what really happened, the idea that wrong instructions may be fetched or the FPU getting stuck doesn't sound good to me. The previous design certainly allowed for a possibility of such conditions being met.

I'll upload a clean copy of OPL here later, for you to pass to him to test. At some point, I broke VMC through a typo, but the current head commit should have no problems as all pull requests have been merged.

Jay-Jay-OPL commented 5 years ago

@ElPatas1

Just for reference to what SP193 stated above, I've been compiling the ifcaro repo with the most current ps2-packer build -- so in case you are not doing that, you may want to go ahead and update that on your compiling environment. -- That's if that user that is having this issue may be using your compiled versions.

Now if the user is getting the ifcaro repo from the auto-bot from @AKuHAK 's script, make sure to let him know that currently that is broken, it's not compiling the recent builds. It's stuck at a very old build: Open PS2 Loader 0.9.3+.1152-Beta-58a493e -- So make sure to verify that.

@SP193, though on my OPL Settings, I do have the Remember Last Played Game enabled, but then I do have the auto-run feature disabled (set to 0). So perhaps, I haven't noticed a problem with that setting??

TnA-Plastic commented 5 years ago

You are right... It was a 9-code, instead of an F-code. I accidentally switched them...

Nope... You can simply change those from F to 9 and vice versa... It just depends on the cheat-program, if it supports either... At least that's how I remember it! There are/were quite some threads about CB + HDLoader or OPL which talk about that kind of topic (F- vs. 9-code). ;) They should be from ~2008-2010 and specifically on psx-scene.com I will look for a link...

As far as I remember, Mastercodefinder does simply automatically do, what else would be done via PS2Dis to find the function where the cheat is hooked to... The function which triggers the cheat is defined (it's memory-location) via the Mastercode and every time it is called, it changes the specified cheat-location-value to the specified value/string.

So... If you choose to use a function which is called quite often, it will keep a value (like unlimited life, etc.) and if you use a function which is only used during certain events (like opening a door), it will only be triggered in that case.

AFAIK, Mastercode-Finder only finds the function which is ever called. I suppose it is/was something related to the VMode, like an event which happens every frame... Something like the VBlank or so?!

So... Essentially PS2RD could support both kind of codes (and maybe it does), but I suppose one should not mix them, but only use one kind of 'system'.

I agree, that this issue seems to be a user-error!

ElPatas1 commented 5 years ago

I asked him again,

he knows the mastercode needs start with 9,and with the second master code in principle it seems to be indifferent, the game works well with the cheat activated or not activating that second master code.

In these beta revisions the OPL is hung up if you launch it from the PS2rd independently active or not codes for a specific game.

He tried the version of the beta that it puts (PADEMU + HIRES) (OPNPS2LD r1166) and it is also indifferent that you launch the OPL from mass or from the memory card (mc?) In the PS2rd, actives or not cheats in this one, just in the moment you launch the game in the OPL it hangs, frozen image in saving configuration and from there do not pass.

He use internal HDD, and do not use the VMC.

Best regards.

sp193 commented 5 years ago

I don't really know (yet) what that all means, unfortunately. If there is a problem with even booting games (regardless of whether the ps2rd feature is used or not), then it should affect all of us. Here is a pre-built copy of the latest commit: https://www.sendspace.com/file/z2bi1x

TnA-Plastic commented 5 years ago

Any news on that front? Did he try some other versions?

Is he sure, that it was the version which implemented these features permanently, or could it be a revision where some memory-stuff has changed, or etc.?

ElPatas1 commented 5 years ago

I'm waiting for his response, i think that the user used the ps2rd externally and not the integrated one, and probably the problem he has is with the external ps2rd only.

When i receive the response i confirm this.

Best regards.

ElPatas1 commented 5 years ago

@sp193, the user tested you pre-built copy and worked fine, like other versions with the integrated ps2rd.

He was definitely using an external version of the ps2rd which has the problem, i thought the user was talking to me about the integrated version, sorry for the inconvenience.

I suppose if the integrated version works well, there is not much need for investigate why the external version has stopped working with the OPL.

I told him to try other versions of the external ps2rd or use the integrated version.

Best regards.

TnA-Plastic commented 5 years ago

So appearantly, it could be an issue with multiple TSR-Routines or 'reloading' of them, or similar... (possibly due to how it got permanently integrated?!)

I however think it is a non-issue, except if it also causes issues on reboots (like via IGR)! ;)

If someone could test this and there is no issue after reloads + those changes would be merged (if they address the culprit), then this issue could be considered 'closed' I think!

TnA-Plastic commented 5 years ago

So appearantly there are still some issues, or are they fixed or a user-issue? It seems, some pull-request regarding this issue are still awaiting a merge, or is this issue (here) fixed?