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

Overhaul memory card selection and folder creation code #308

Merged
merged 3 commits into from
Aug 14, 2020

Conversation

uyjulian
Copy link
Member

@uyjulian uyjulian commented Aug 1, 2020

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

Fixes OPL directory creation on memory card.
Supersedes #307
Closes #302

@J013k
Copy link
Contributor

J013k commented Aug 1, 2020

I made a quick test.
While creating OPL seems to work for MC... I now have a different problem.

If I will pull out MC from 1st slot, have only MC in 2nd slot. Save settings.
Settings will be saved to mc1.

When I will put MC in 1st slot and I want to save settings.
Setting will be created\saved on mc0.

Here is my problem.
Settings supposed to be updated on mc1, not newly created on mc0.

So with 2 MC, setting will always be saved on MC in 1st slot, no matter if settings already exists on MC in 2nd slot.

@uyjulian
Copy link
Member Author

uyjulian commented Aug 1, 2020

The logic is roughly the following:
If slot 0 contains "OPL" folder, it will be selected.
If slot 1 contains "OPL" folder, it will be selected.
If slot 0 is inserted and is a PS2 format card, it will be selected.
If slot 1 is inserted and is a PS2 format card, it will be selected.
After the selection is done, it won't be checked again until the program is restarted.

I believe this is the correct logic for memory card selection.

@J013k
Copy link
Contributor

J013k commented Aug 1, 2020

If slot 1 contains "OPL" folder, it will be selected.

Here is my problem.
With this PR, OPL is not looking for OPL folder in mc1.
It will always save settings into mc0.

EDIT:
I have also notice that If I have only setting on mc1.
OPL will not load them if there is MC in slot 1 (mc0)
With only one MC in 2nd slot, while loading setting I'm getting -> CNF loaded from mc
It should be -> CNF loaded from mc1:

@uyjulian
Copy link
Member Author

uyjulian commented Aug 1, 2020

I don't have multiple memory cards to test, so let me know if this latest push fixes the issue for you.

@J013k
Copy link
Contributor

J013k commented Aug 1, 2020

Thank you.
This fix seems to help, but I have another problem...
While saving setting I'm getting:

  • Settings saved to mc0:
  • Settings saved to mc -> "1:" is missing for memory card in 2nd slot

While loading settings I"m getting:

  • CFG loaded from mc -> I don't know if settings were loaded from "mc0:" or from "mc1:".

@uyjulian
Copy link
Member Author

uyjulian commented Aug 1, 2020

Let me know if the 1: is missing or not

@uyjulian
Copy link
Member Author

uyjulian commented Aug 2, 2020

Okay, the 0: and 1: should be displaying now.

@J013k
Copy link
Contributor

J013k commented Aug 2, 2020

While saving settings everything seems to be fine:

  • Settings saved to mc0:
  • Settings saved to mc1:

But not while loading.
I still getting:

  • CFG loaded from mc

Instead of:

  • CFG loaded from mc0:
  • CFG loaded from mc1:

EDIT: I need to re-check it, because you have uploaded new version.
EDIT 2: Even with OPNPS2LD-0.9.3+.1550-Beta-3d3cebe.ELF I have the same problem.

@J013k
Copy link
Contributor

J013k commented Aug 3, 2020

New version\commit works for me.
I mean:

  • If there is no OPL folder on mc0 while saving setting it will be created.
    Also I will get message\notification Settings saved to mc0:

  • If there is no OPL folder on mc1 while saving setting it will be created.
    Also I will get message\notification Settings saved to mc1:

  • If there is no OPL folder on mc0\mc1 while saving setting it will be created.
    Also I will get message\notification Settings saved to mc0:.
    Because mc0 has got higher priority.

While loading setting I'm getting:

  • CFG loaded from mc0:
  • CFG loaded from mc1:

I'm only curious when OPL is looking for OPL folder?
During boot, or while saving settings?
Probably during both operations, because how I'll get Settings saved to mc0:.
Or it is just looking for conf_opl.cfg during boot, and checks for OPL folder while saving settings?

I'm asking, because currently with 3rd party MC, booting I think is taking ~1\2 s more time.
I may be paranoid...

I also need to check if priority system works.
mc0 -> mc1 -> hdd0 -> ETH -> USB \ mc0 -> mc1 -> hdd0 -> USB -> ETH

@uyjulian
Copy link
Member Author

uyjulian commented Aug 3, 2020

The folder is only checked/created when saving settings.

@J013k
Copy link
Contributor

J013k commented Aug 3, 2020

Hmm this is strange.
I have taken some time to check time to boot OPL with 3rd party MC.
With OPL 1442 it is taking ~18 s to see OPL logo,
with OPNPS2LD-0.9.3+.1551-Beta-b0cd20f.ELF almost ~32 s.

Settings were loaded from HDD.
Both OPL version was loaded from mc1 (8 MB genuine MC).
3rd party MC was in 1st slot (mc0).

@uyjulian
Copy link
Member Author

uyjulian commented Aug 3, 2020

I removed the additional memory card checks, so let me know if the loading time improves.

@J013k
Copy link
Contributor

J013k commented Aug 3, 2020

Loading time has been improved.
Currently it is taking ~22 s.

It takes 18 s in OPL 1442 to see logo.

So 18 s vs. 22 s (previously 32 s).

@J013k J013k mentioned this pull request Aug 3, 2020
@J013k
Copy link
Contributor

J013k commented Aug 3, 2020

Probably last question.
I thought that priority system while saving looks like:
mc0 -> mc1 -> hdd0 -> USB
I don't know why but USB currently has got higher priority:
mc0 -> mc1 -> USB -> hdd0

OPL was launched from hdd0.

@KrahJohlito
Copy link
Member

@J013k https://github.com/ps2homebrew/Open-PS2-Loader/blob/master/src/opl.c#L627
It has been that way for a long time, mc->USB->hdd

@zappepappe
Copy link

From 5443a19

Changed how devices are selected for loading/storing config files.

When loading:

  1. Check memory cards.
  2. If config could be loaded, try the device that OPL was booted from (supported devices only).
  3. If config could not be loaded, try all supported devices.
  4. Default to memory card, if no config file could be loaded.

When saving:

  1. Try the device that the config file was loaded from.
  2. If config could not be saved, try the device that OPL was booted from (supported devices only).
  3. If config could not be saved, try all supported devices.
  4. Give up.

@J013k
Copy link
Contributor

J013k commented Aug 4, 2020

Thanks @zappepappe & @KrahJohlito.

It makes me wonder a bit:

When saving:
2. If config could not be saved, try the device that OPL was booted from (supported devices only).

From what I remember in my scenario OPL was launched from HDD.
There were no MC. Only HDD+USB.
I additionally initialize HDD, games list was shown.
Settings were saved to USB (Settings saved to mass:).

@KrahJohlito
Copy link
Member

KrahJohlito commented Aug 12, 2020

@J013k are you able to test other revisions to see where this behaviour changed? I would suggest trying r1443 then r1444 if it’s not caused by this commit (which I don’t think it is).

@J013k
Copy link
Contributor

J013k commented Aug 12, 2020

Currently I'm far away from home.
I have only SCPH-77004 to test.
The earliest date that I can make this test on SCPH-50004 (with HDD)
is somewhere around 21\22.08.2020...

@J013k
Copy link
Contributor

J013k commented Aug 14, 2020

Additionally can someone try to test without any additional device with only memory card(s) this build
and compare how long will it take to see OPL logo with others (e.g. OPL 1555, 1442) builds?

From my test with genuine MC (boot sound enabled) it takes ~10 sec to see OPL logo in OPL 1555.
~12 sec in current test build (Overhaul memory card selection and folder creation code).

Results with 3rd party MC may also be different:
#308 (comment).

OPL loaded (ELF + config) from MC.

As for save settings priorytety I can make an another issue. Because this PR seems to fix root of a problem...
However I don't know if someone else have different OPL loading times... (this build vs other).

@uyjulian
Copy link
Member Author

The boot time may have changed because the memory card is being checked.
The memory card was not checked properly in previous builds.

@J013k
Copy link
Contributor

J013k commented Aug 14, 2020

Would you like to continue discussion about save setting priority here,
or to create separate issue?
If to create different issue, everything seems to be fine here.

@uyjulian
Copy link
Member Author

I think creating separate issue should be good. I'll merge this.

@uyjulian uyjulian merged commit 36ebf7b into ps2homebrew:master Aug 14, 2020
@J013k
Copy link
Contributor

J013k commented Aug 14, 2020

Thank you. I will try to create separate issue in free time.

Best regards.

AKuHAK pushed a commit that referenced this pull request Sep 30, 2021
Overhaul memory card selection and folder creation code
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
Overhaul memory card selection and folder creation code
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.

Cannot save setting on MC
4 participants