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

[FR] Enhance HDD configuration location #358

Closed
AKuHAK opened this issue Jan 9, 2021 · 12 comments
Closed

[FR] Enhance HDD configuration location #358

AKuHAK opened this issue Jan 9, 2021 · 12 comments

Comments

@AKuHAK
Copy link
Member

AKuHAK commented Jan 9, 2021

Is your feature request related to a problem? Please describe.
For now, OPL is creating partition "+OPL" on launch. First of all it is wastingof free space on hdd (128 Mb). And maybe someone wants to use different location for storing configuration.

Describe the solution you'd like
Maybe it will be useful to move configuration into "__common/OPL"? For those who still want to use old "+OPL" maybe can be used some file inside "__common/OPL" which contains alternate partition name.

Personally, I am using " PP.OPL" partition to make it visible through HDD Browser as well.

The fix can be implemented really easy, but I dont know maybe I missed aomething.

@israpps
Copy link
Member

israpps commented Jan 10, 2021

what about adding a new config:
Partition Name: (allowing the user to choose between +OPL an PP.OPL for the Partition name)

and what about adding a mesage if the HDD partition doesnt exist?? allowing the user to choose the partition size (instead of creating +OPL @128 Mb without asking)

@TnA-Plastic
Copy link

TnA-Plastic commented Jan 10, 2021

That's what he suggested in other words.
I like the "size-choice"-idea though.

It's a good idea, because a standard-format of the HDD includes "__common".
Was that partition used for "saves" as well?

I think the best solution would be to have support for an OPL CFG in there, but let it "fall back" to +OPL, once it doesn't find a config there.

I suppose that might even be only a line for the lookup-path to add, hence (if I'm correct) a matter of seconds to implement.
If an "icon" should be implemented, maybe longer...

Regarding the size-choice.
I thought about a "setup-routine" to show up if no config is found. That could have a "size-choice" for "+OPL" as well.

@israpps
Copy link
Member

israpps commented Jan 11, 2021

That's what he suggested in other words.
I like the "size-choice"-idea though.

It's a good idea, because a standard-format of the HDD includes "__common".
Was that partition used for "saves" as well?

I think the best solution would be to have support for an OPL CFG in there, but let it "fall back" to +OPL, once it doesn't find a config there.

I suppose that might even be only a line for the lookup-path to add, hence (if I'm correct) a matter of seconds to implement.
If an "icon" should be implemented, maybe longer...

Regarding the size-choice.
I thought about a "setup-routine" to show up if no config is found. That could have a "size-choice" for "+OPL" as well.

yes! but instead of writing the partition name, choose between fixed options.
also, keeping config inside __common\OPL\ * doesnt seem to be a good option, since it can be erased by accident from HDD-OSD.
instead it should be in the OPL config file, (or if you want to keep it in a fixed place inside HDD... __sysconf\OPL* seems to be a litle bit safer than __common, that way, only uLaunchELF and PfsShell will be capable of affecting that .cfg file

@AKuHAK
Copy link
Member Author

AKuHAK commented Jan 11, 2021

also, keeping config inside __common\OPL\ * doesnt seem to be a good option, since it can be erased by accident from HDD-OSD.

There can be many options that can be done "by accident" I don't think that we should rely on it. The user can delete "by accident" memory card folder which contains opl settings and so what? Should we keep that folder protected or hide it, or do some user protection for it? I don't think that this can be a reason for not using it for storing configuration.
__common partition was created by Sony with the aim of storing configurations per game. It is the biggest partition of standard ones.

__sysconf used for storing HDD OSD supported files and system drivers. FHDB can be treated as a system driver as well - this is why it is stored there. OPL isn't a system driver - it is an app.

@TnA-Plastic you want to add also some fancy GUI option with user-selectable options? It will be cool but it will require a lot of work. As for now I just advise storing the partition name inside some config file and that's all. If there is no such file it will use __common/OPL. As about size, maybe we can add size inside config file.

@KrahJohlito
Copy link
Member

KrahJohlito commented Jan 21, 2021

@AKuHAK
what about something like this KrahJohlito@b1ca359 ?

Basically when hdd is init it checks __common for an OPL folder, then if PP.OPL partition exists, then if +OPL partition exists.. if all fail a window pops up asking where you wanna make your opl part (up and down to cycle through the 3 selections)
hdd

Then upon save, stores it in your opl cfg to know which to mount at next boot. I've only been able to test on pcsx2.
https://github.com/KrahJohlito/Open-PS2-Loader/actions/runs/501077999

Additionally could add a gui option if people have multiple opl parts (for some reason?) and want to change which one they're using without having to delete cfgs or partitions.

Edit: pushed some changes.

@AKuHAK
Copy link
Member Author

AKuHAK commented Jan 21, 2021

I did a quick review: looks very promising.

Some notes:
please unmount "pfs0" before mounting the HDD partition. There is a very high possibility that this mount point will be already mounted by some other function or et even by the app from where OPL is loaded. Just every time you mount - umount pfs0: before.

All OPL filesystems use the root folder by default. I hope that using __common:/OPL (not root) folder will not break something. I hope that OPL isn't using functions like "go to the root of device and do some stuff" but is using "go to the configuration folder and do some stuff". Otherwise __common can be easily filled with opl trash and it is possible that something useful will be deleted. It is better to test the subfolder on some isolated partition before. Or even better use OPL subfolder always, it even on +OPL partition (so +OPL/OPL is used).

By the way, subfolder support will be good stuff for USB too. I really don't like all these folders (DVD, CD, CHEATS, etc) at the root of my USB.

One more thing. __common option in GUI should be visible as a different option (if we create a subfolder only there). And this option should be the "default" option for selection (as preferable).

Didn't quite understand from the code how it will handle already created partitions if we try to choose one.

As I understand your logic now is such:
"load partition name from config that was loaded at the boot"
"if no such config - try if exist folder __common/opl"
"if no folder, try 2 predefined partitions"
"if no partitions, create a new one"

The problem is - where is saved this config? If it will be saved on a custom location, how opl will know on the next boot where to get it? For example, if only 1 HDD is attached to ps2, no USB, no memory cards, nothing except hdd, then opl will ask for creating new partition every time on boot.

My idea was, that __common/OPL/partition.txt is a static path at least for HDD location. So opl will load that one, read partition name from it, and then use for other settings that second partition.

@israpps
Copy link
Member

israpps commented Jan 21, 2021

I did a quick review: looks very promising.

Some notes:
please unmount "pfs0" before mounting the HDD partition. There is a very high possibility that this mount point will be already mounted by some other function or et even by the app from where OPL is loaded. Just every time you mount - umount pfs0: before.

All OPL filesystems use the root folder by default. I hope that using __common:/OPL (not root) folder will not break something. I hope that OPL isn't using functions like "go to the root of device and do some stuff" but is using "go to the configuration folder and do some stuff". Otherwise __common can be easily filled with opl trash and it is possible that something useful will be deleted. It is better to test the subfolder on some isolated partition before. Or even better use OPL subfolder always, it even on +OPL partition (so +OPL/OPL is used).

By the way, subfolder support will be good stuff for USB too. I really don't like all these folders (DVD, CD, CHEATS, etc) at the root of my USB.

One more thing. __common option in GUI should be visible as a different option (if we create a subfolder only there). And this option should be the "default" option for selection (as preferable).

Didn't quite understand from the code how it will handle already created partitions if we try to choose one.

As I understand your logic now is such:
"load partition name from config that was loaded at the boot"
"if no such config - try if exist folder __common/opl"
"if no folder, try 2 predefined partitions"
"if no partitions, create a new one"

The problem is - where is saved this config? If it will be saved on a custom location, how opl will know on the next boot where to get it? For example, if only 1 HDD is attached to ps2, no USB, no memory cards, nothing except hdd, then opl will ask for creating new partition every time on boot.

My idea was, that __common/OPL/partition.txt is a static path at least for HDD location. So opl will load that one, read partition name from it, and then use for other settings that second partition.

what about skipping __common completely?
make OPL read PP.OPL, if it exist, use it, if it doesnt exist, then try with +OPL. if none of them exist display Partition creation dialog

@KrahJohlito
Copy link
Member

please unmount "pfs0" before mounting the HDD partition. There is a very high possibility that this mount point will be already mounted by some other function or et even by the app from where OPL is loaded. Just every time you mount - umount pfs0: before.
Sure no problem.

All OPL filesystems use the root folder by default. I hope that using __common:/OPL (not root) folder will not break something. I hope that OPL isn't using functions like "go to the root of device and do some stuff" but is using "go to the configuration folder and do some stuff". Otherwise __common can be easily filled with opl trash and it is possible that something useful will be deleted. It is better to test the subfolder on some isolated partition before. Or even better use OPL subfolder always, it even on +OPL partition (so +OPL/OPL is used).
It “shouldn’t” be a problem, this is why I added the gHDDPrefix variable for things like saving cfg loading games, cheats, making folders etc. Where as mounting, Unmounting etc I’ve left the old hddprefix (“pfs0:”) because for __common the former uses need to be done at pfs0:OPL/.
It would be good if this could get tested though.

I like the idea of a sub folder for every partition and even USB but because it’s been used the old way for so long if we change it now it will break compat for lots of people so I’d rather just leave it.

As I understand your logic now is such: "load partition name from config that was loaded at the boot" "if no such config - try if exist folder __common/opl" "if no folder, try 2 predefined partitions" "if no partitions, create a new one" The problem is - where is saved this config? If it will be saved on a custom location, how opl will know on the next boot where to get it? For example, if only 1 HDD is attached to ps2, no USB, no memory cards, nothing except hdd, then opl will ask for creating new partition every time on boot.

Yea that’s correct, the config is saved on your newly created partition (or folder in __commons case) manually by selecting “Save Changes” in the menu. If it is the only device connected, then on next boot gOPLPartition is default to +OPL (since that’s what it has been for years) so it checks there first then the others, so you won’t be asked to make a partition every time. If one already exists it will be found.

P.S excuse my use of the code brackets here I can’t find the quote button on my phone :-P

@KrahJohlito
Copy link
Member

Yea it might be better to just skip __common, it brings a lot more complications than just using PP.OPL & +OPL.. wouldn’t have to worry about different prefixes or folders.. code would be cleaner too.. I’ll make some changes when I can. 👍

@AKuHAK
Copy link
Member Author

AKuHAK commented Jan 22, 2021

But if you skip the __common partition you will loose the main goal: saving hdd space. PP.OPL or +OPL both are 128Mb. I agree that PP.OPL is better than +OPL for those who are using HDD OSD cause they will save 1 partition. But for others it is still wasting of space, this is why I advice to move default location to the __common/OPL

Regarding pfs it is seems that you misunderstand the pfs logic. So you call filexiomount function, then you open pfs0. But you cannot be sure that filexiomount will mount exactly pfs0. Filexiomount will mount the lowest possible partition. So if pfs0 is taken it will mount pfs1, if pfs0and pfs1 are taken itwill mount pfs3. You cannot be sure that there were no filexiomount calls before your function called it. So it will be much safer to umount pfs0 each time you try to mount it (not only in the end).

@KrahJohlito
Copy link
Member

But if you skip the __common partition you will loose the main goal: saving hdd space. PP.OPL or +OPL both are 128Mb. I agree that PP.OPL is better than +OPL for those who are using HDD OSD cause they will save 1 partition. But for others it is still wasting of space, this is why I advice to move default location to the __common/OPL

Fair point.

Regarding pfs it is seems that you misunderstand the pfs logic. So you call filexiomount function, then you open pfs0. But you cannot be sure that filexiomount will mount exactly pfs0. Filexiomount will mount the lowest possible partition. So if pfs0 is taken it will mount pfs1, if pfs0and pfs1 are taken itwill mount pfs3. You cannot be sure that there were no filexiomount calls before your function called it. So it will be much safer to umount pfs0 each time you try to mount it (not only in the end).

I'm not sure I explained what I meant very well. "Sure no problem." means Yes I can do that :-P.

When OPL is doing things like making folders etc it uses the variable hddPrefix which is fine when everything is being done on root but when __common is used as the opl partition we don't want it done on root so I added another variable gHDDPrefix but also kept the original hddPrefix variable for unmounting since even though we arent working on root of __common we still want to mount/unmount pfs and not pfs0:OPL/ if that makes sense.
So I don't think there will be any problems using a sub folder only in this case.

I've made some changes (keeping __common) KrahJohlito@edc62c4

Unmounting pfs0 before mounting.
Prompt now reads "Which HDD partition would you like to use?" to better suit all scenarios.
Also in the previous build I was inadvertently formatting __common before making the OPL dir :-s so thats fixed now also.

@AKuHAK
Copy link
Member Author

AKuHAK commented Jul 6, 2021

fixed in #423

@AKuHAK AKuHAK closed this as completed Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants