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

Fix MC icon writing and small improvements #223

Merged
merged 2 commits into from
Sep 30, 2019

Conversation

Tupakaveli
Copy link
Contributor

@Tupakaveli Tupakaveli commented Sep 29, 2019

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

  • Notifications code improvement.

  • Fix MC icon writing and OPL folder creation.
    MCs will be checked for OPL folder at boot, if not found OPL will check if MCs are inserted. If MC is inserted, OPL folder and MC icon will be created when saving config. If OPL folder already exists, opl.icn and icon.sys will be checked. If either or both do not exist, both will be written. When saving config to another device, OPL folder will not be created on MC.
    This fixes issue Generate icon when saving configuration #124

  • Allow AttributeText and AttributeImage to be used on Main page of themes.

Test build: https://www.sendspace.com/file/al2e18
Test theme: https://www.sendspace.com/file/36klio

KrahJohlito and others added 2 commits September 29, 2019 12:17
* mc: OPL folder and mc icon will now be checked and written when saving config to mc, if not found..

* gui: improve notifications code, slightly
@J013k
Copy link
Contributor

J013k commented Sep 29, 2019

In current build memory card has got highest priorytety.
I mean, when there is no settings on any device, settings with OPL icon will be saved onto mc0:,
no matter if OPL were loaded from mass:, mc1:, probably also hdd0:.
The same thing is with loading.
When OPL will be loaded from mass:, settings will be on mc?: and mass:,
settings will be loaded from mc?

So the need to copy setting using e.g. wLe from mc0 to mc1 is needed.
Workaround for that is to pull out all devices except the one you need for setting.

In current build everything with saving & generating config should be fine.
I mean, when I've setting on mass:, setting will be saved on mass:.
OPL will not create additional OPL folder on memory card.

When settings are on mc1:, setting will be saved on mc1.

I haven't tried HDD because currently I've no access to my other PS2 with NA.

I know that new theme is a test version, so maybe little feedback will be useful.
When I load this theme, loading\working image is disabled.
I mean this icon:
http://psx-scene.com/forums/attachments/f150/59393d1504712481-open-ps2-loader-project-v0-9-3-32x32.gif

I happens at least at games list (today I've little time for more tests).
When I switch through few devices (e.g. from USB to ETH),
icons showing devices are in some way broken.
I mean when I'm at USB device I've all icons (USB, ETH, HDD),
when I'll switch into ETH only this icon will be shown.

@Tupakaveli
Copy link
Contributor Author

About the theme, if you have SFX enabled the loading icon is disabled during intro/splash screen because of the delay to time the end of the boot sound with the logo fade. Otherwise the loading icon appears to stall.

Also with the device icons, I was testing with PCSX2 before testing on real hardware so I didn't bother including the icons other than USB. It's just meant to show that AttributeText and AttributeImage work on Main page.

@KrahJohlito
Copy link
Member

KrahJohlito commented Sep 29, 2019

Took me a lil while to figure out what could be the cause, I think what’s happening is sysCheckMC is having the draw back of upon checking if mc is inserted... of then making mc the current working device before OPL has a chance to check for config from boot device.. thus ruining @sp193 s work with boot device priority... that is... unfortunate.

Given that this is the case I don’t think we can have it both ways... at least I can’t think of an alternative at the moment, I will revert the way OPL handles folders on MC but leave the MC Icon fix in place and that should restore boot priority, but unfortunately we will have to live with empty folders on MC when booting from other devices for the mean time.

I apologise for the inconvenience.

Best regards

@J013k
Copy link
Contributor

J013k commented Sep 29, 2019

I just wanted to write how currently loading and saving config is handled.
More info is here:
https://www.psx-place.com/threads/open-ps2-loader-game-bug-reports.19401/page-13#post-149544.

I mean everything works as it should.

Back in the days I wanted to user decide where setting suppose to be saved,
something like with FMCB\FHDB:
https://s33.postimg.cc/5sy389xmn/save_OPL.png
Unfortunately:

  • Theoretically it can confuse new users, because not every ones know what is mc1, mass etc.
  • Another function will be needed to add (another complexity).
  • Handling OPL suppose to be easy

@KrahJohlito
Copy link
Member

KrahJohlito commented Sep 30, 2019

I mean everything works as it should.

Wait, sorry I got confused... It seemed as though you were saying there was a problem with the pull request.. I have only been testing on pcsx2 so I can’t test boot priorities (not in relation to where OPL was booted from anyway), @Tupakaveli is going to test and if nothing is broken or changed from how OPL already operates other then what is intended by the changes then this pull request can go through. Otherwise I will revert the MC folder handling.

It now seems as though you were simply saying it still works the same way but it’d be good if it were different?.. there is some reason sp193 made it not go into alternate save mode on first boot, I’m not exactly sure of the reason but I’m sure it’s a good one.. so I don’t want to mess with it.

@sp193
Copy link
Contributor

sp193 commented Sep 30, 2019

Part of the design is like that, as the code was originally like that. Changing the boot order may affect some users, even though the original order may have some drawbacks.

The "alternative" boot modes are as such because OPL used to only support memory cards. They were already called as such, long before I started working on OPL. I only changed the code so that a device can be selected by OPL at the point of saving, even if one was not available immediately upon boot.

The only time when the folders have to exist, would be during saving. It does not matter if they do not exist, if the user does not use that device. If the folders are always created on the memory card, then that is an effect of old code; the check function would always create folders, even if they were never required.

@Tupakaveli
Copy link
Contributor Author

Tested and the cfg saving/loading behaviour has not changed. Only the folder creation and MC icon writing behaviour has changed.

Best regards.

@KrahJohlito
Copy link
Member

@ElPatas1
Sorry for the confusion.

This pull request is fine and complete, I misunderstood joleks comment.

@J013k
Copy link
Contributor

J013k commented Sep 30, 2019

Yes it's fine.
I've added some confusing extra data about loading and saving settings.
Which is not fully related to this pull request.

Sorry about confusion that I've made.

@ElPatas1 ElPatas1 merged commit f112b26 into ps2homebrew:master Sep 30, 2019
@Jay-Jay-OPL
Copy link
Contributor

Jay-Jay-OPL commented Oct 1, 2019

@Tupakaveli and @KrahJohlito Just a little tip.

When writing the commit description, try avoiding putting a hashtag and a number together (example: #14).

Why? It will cause GIT to link and refer to an older commit number within OPL, but then it causes confusion for us (i.e. changelogs, commits outline, and etc.), since the new commit really doesn't relate to that older commit.

So instead try writing it as: 14# or something else, so GIT doesn't do that. Only write it that way, when you really want to point to an older commit you guys are either updating or referring to (some changes made to it or etc).

I hope this tip helps you out... ;)

@Tupakaveli
Copy link
Contributor Author

@Jay-Jay-OPL it was not manually written. Github added it automatically because @KrahJohlito pull requested to my fork before this pull request was opened.

You can use the git hash to view the actual commit.

@Jay-Jay-OPL
Copy link
Contributor

Oh, I see. Seems like a bug with GIT then, since those hyperlinked numbers should point to that repo, then the one it now got merged into. I wonder if there is a way to avoid that, if not, well, hopefully GIT fixes that "somehow" on their end.

AKuHAK pushed a commit that referenced this pull request Sep 30, 2021
Fix MC icon writing and small improvements
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
Fix MC icon writing and small improvements
This pull request was closed.
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.

6 participants