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

Small improvement to IGR & Remove unneeded option. #378

Merged
merged 2 commits into from
Jan 31, 2021
Merged

Small improvement to IGR & Remove unneeded option. #378

merged 2 commits into from
Jan 31, 2021

Conversation

KrahJohlito
Copy link
Member

@KrahJohlito KrahJohlito commented Jan 24, 2021

Pull Request checklist

Note: these are not necessarily requirements

  • I reformatted the code with clang-format
  • I checked to make sure my submission worked
  • I am the author of submission or have permission from the original author
  • Requires update of the PS2SDK
  • Requires update of the gsKit
  • Others (please specify below)

Pull Request description

partially fulfils #230 - Using sp193s suggestion, IGR will now work booting exit ELFs stored on USB drives (exit path should be "mass:app.elf").. however it will only work if a FMCB card is attached as the modules are being loaded from there.. I'm not sure about HDD & SMB I don't think they are really viable options for IGR as we'd need to not only load the modules but also mount a specific partition or connect to an smb share.

Also I removed the option to check if USB games are fragmented and instead this check will always happen, just seems like a pointless option to me.. I see no reason for this check to be turned off and can only lead to incorrect compatibility reports.

@AKuHAK
Copy link
Member

AKuHAK commented Jan 24, 2021

@KrahJohlito you mixed different problem solutions in one PR, it will be more difficult to review your changes. Anyway...
About HDD
I see more problems with custom partitions. If you choose some non-standard partition name (for example PSX DESR users have to use longer partition names with XMB - PP.OPSL-00000 as an example) OPL configuration file will be saved in that custom partition. But on the next boot, this configuration will be skipped.
I see some solutions for this:

  • If you allow booting configuration only from 3 different places on HDD, you should allow saving also only in these 3 different places.
  • You can allow only these 3 different partitions for use. You can also remove the GUI option as only 3 partitions are used, and the user has no control from GUI on them.
  • You can use static __common/OPL for storing gHDDPrefix only.
  • It is also possible to change the behavior of opl a lot by parsing arguments that are sent to OPL.ELF on boot. I mean making OPL fully portable - so it will use the path that it is booted from for loading configuration.

@KrahJohlito
Copy link
Member Author

I see more problems with custom partitions. If you choose some non-standard partition name (for example PSX DESR users have to use longer partition names with XMB - PP.OPSL-00000 as an example) OPL configuration file will be saved in that custom partition. But on the next boot, this configuration will be skipped.

The dialog window that appears does not allow the user to input whatever partition they like, it’s a choice of 3 partitions.. I suppose they could manually edit the cfg file to mount any partition; is that what you mean?

Current available selections:
__common (Will create OPL folder if selected so we aren’t working on root here)
PP.OPL
+OPL

By default the variables are set to +OPL & pfs0: since that’s OPLs old default settings, only difference really is now they aren’t forced.

  • If you allow booting configuration only from 3 different places on HDD, you should allow saving also only in these 3 different places.

That’s pretty much how it is already, unless the cfg is manually edited as per my last reply.. To avoid this the cfg entry could just be enumerated rather than a string I guess?

  • You can also remove the GUI option as only 3 partitions are used, and the user has no control from GUI on them.

There is no GUI option only a cfg entry (unless you’re talking about the dialog window).

  • You can use static __common/OPL for storing gHDDPrefix only.

gHDDPrefix can’t really be static because it needs to be different depending on whether we’re working on root or not, but I could be misunderstanding what you mean.

@AKuHAK
Copy link
Member

AKuHAK commented Jan 25, 2021

I see more problems with custom partitions. If you choose some non-standard partition name (for example PSX DESR users have to use longer partition names with XMB - PP.OPSL-00000 as an example) OPL configuration file will be saved in that custom partition. But on the next boot, this configuration will be skipped.

The dialog window that appears does not allow the user to input whatever partition they like, it’s a choice of 3 partitions.. I suppose they could manually edit the cfg file to mount any partition; is that what you mean?

Current available selections:
__common (Will create OPL folder if selected so we aren’t working on root here)
PP.OPL
+OPL

By default the variables are set to +OPL & pfs0: since that’s OPLs old default settings, the only difference really is now they aren’t forced.

I thought that the user should type partition name now when the GUI dialog appeared. So he can type whatever he wants, not only 3 partition names. If the user is limited to 3 scenarios only it is good enough for me.

  • You can use static __common/OPL for storing gHDDPrefix only.

gHDDPrefix can’t really be static because it needs to be different depending on whether we’re working on root or not, but I could be misunderstanding what you mean.

I mean that the gHDDPrefix value is stored in an additional config file which is placed in a static place always. For example :
the file __common/OPL/gHDDPrefix.txt which contains PP.OPSL-0000/OPL, or contains +OPL, or contains __common/OPL, or whatever user choose. When OPL boots, it will try to boot that config file first, reads gHDDPrefix value, and then change location. But at boot, it will always check this file __common/OPL/gHDDPrefix.txt.

@KrahJohlito
Copy link
Member Author

@AKuHAK
This is how it is currently in this PR.

OPL1.mp4

This is a quick PoC of how it could be if we allow any partition to be entered. I haven't bothered changing any of the cfg handling here cause we'd need a seperate cfg in __common/OPL like you suggested and would need to determine if the user entered sub folders etc. If you prefer it this way I could do it but currently lack motivation so it might take a few days.

OPL2.mp4

Copy link
Member

@rickgaiser rickgaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment on the PFS/HDD changes as I know too little about it. But @AKuHAK seems to know a lot about it so he's probably the best to review/approve those changes.

The other changes look good, so approved.

@rickgaiser
Copy link
Member

It is also possible to change the behavior of opl a lot by parsing arguments that are sent to OPL.ELF on boot. I mean making OPL fully portable - so it will use the path that it is booted from for loading configuration.

One of the problems with ps2 in general is that all applications reboot the IOP, and are responsible for loading their own device drivers. For example: mass0 on wLE could be USB, while mass0 on OPL could be mx4sio. Even worse, OPL could be loaded from a location OPL cannot access at all, like host.

One solution would be to do it like it's supposed to be done on the PS2: save the configuration onto a memorycard. Preferably in a folder of it's own, with a nice 3d icon and some text.
then from the ps2 os you can copy/delete this "savegame", just like any other samegame.

@TnA-Plastic : A next step would be to also include the ELF in the same folder, and have FMCB boot OPL from there, then call it "App System" ;).

@AKuHAK
Copy link
Member

AKuHAK commented Jan 27, 2021

@KrahJohlito maybe you can remove hdd changes from this pr? So other changes can be accepted.

@rickgaiser oh i didnt know that mx4sio uses the same prefix as usb. In fact Sony had a solution for this, they pass mounting device properties in argv[2] for example

argv[2] = 'hdd0:__common:pfs:/RA/RA.ELF'
This can be changed easily tor bdm style: 'bdm:mc0:Fat32:/OPL/OPL.ELF'

@rickgaiser
Copy link
Member

@KrahJohlito maybe you can remove hdd changes from this pr? So other changes can be accepted.

Good idea.

i didnt know that mx4sio uses the same prefix as usb

It's actually the fat32 file system driver that uses the mass prefix. No matter what block device is connected to it. Originally the fat32 driver would register at the name of the block device, like "usb0", "sd0". But now it is like (for example):

  • mass0 = usb0p1 mounted with fat32 - first partition on usb stick
  • mass1 = usb0p2 mounted with fat32 - second partition on usb stick
  • mass2 = usb1p1 mounted with fat32 - first partition on second usb stick
  • mass3 = sd0p1 mounted with fat32 - first partition on mx4sio
  • etc...

Basically "mass" is now equivalent to "fat32_mountpoint". This will become a new challenge when adding a second filesystem.

@AKuHAK
Copy link
Member

AKuHAK commented Jan 27, 2021

In such logic it should send such params as argv[2]:
bdm:usb0p2:mass:/OPL/OPL.ELF

@KrahJohlito KrahJohlito changed the title Enhance Hdd config location Small improvement to IGR & Remove unneeded option. Jan 28, 2021
@KrahJohlito
Copy link
Member Author

maybe you can remove hdd changes from this pr? So other changes can be accepted.

Ok no problem, I moved it to a different branch and dropped the commit from PR.

@uyjulian uyjulian merged commit c2102be into ps2homebrew:master Jan 31, 2021
@uyjulian
Copy link
Member

I'll go ahead and merge this.

@KrahJohlito KrahJohlito deleted the hdd branch May 24, 2021 06:00
AKuHAK pushed a commit that referenced this pull request Sep 30, 2021
Small improvement to IGR & Remove unneeded option.
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
Small improvement to IGR & Remove unneeded option.
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

Successfully merging this pull request may close these issues.

4 participants