libretro / px68k-libretro

Portable SHARP X68000 Emulator for Libretro
http://hissorii.blog45.fc2.com
GNU General Public License v2.0
45 stars 41 forks source link

"config" file being completely ignored #153

Closed bslenul closed 2 years ago

bslenul commented 2 years ago

Hey!

Currently the retroarch/system/keropi/config file is not being created anymore, and even if you have one from a previous version of the core, it's not being read. So a super useful line like StartDir= can't be used anymore :/

Already bisected, this is the 1st bad commit: 53a1d027ce3e3b991d2a535906b171be7a6a3d4f .

negativeExponent commented 2 years ago

ive doubt this wil even be restored. even loggings are removed. core now broken, buildbot core still even is broken.

inactive123 commented 2 years ago

It should be properly implemented as core options instead of reading from a config file. We dont want all these external config files and we dont want the file I/O that comes with it. should have been done properly first time around.

This core needs heavy work so we have started rewriting stuff as Project IO. We are aiming at the major issues being resolved.

bslenul commented 2 years ago

Might be tricky to implement as core options, no? I believe I've never seen a core with the ability to set a path as a core option, but if that's possible then great, I agree these kind of things should be configurable from RA itself, not externally!

inactive123 commented 2 years ago

We'll likely have to figure something out. I will try to discuss with @jdgleaver what options we have.

inactive123 commented 2 years ago

Can I ask first what this 'starting dir' is used for?

gingerbeardman commented 2 years ago

StartDir= sets the initial directory for the internal file selector that can be used to choose/change disk images whilst the emulator is running.

I guess it could be set to the sae value as the RetroArch ROM directory?

bslenul commented 2 years ago

It's a setting to set the default directory when using the PX68k menu (either by pressing L2 or by starting the core without content), by default on Windows it defaults to C drive but the problem is that, AFAIK, there's absolutely no way to switch drive letter. So for example in my case my roms are in my F drive so the PX68k menu is pretty much useless now.

It's very useful for multi disks games, you can still use .m3u + the "Swap Disks on Drive" core option but for 3 or more disks games but it's a bit awkward IMO, especially for new users.

The removal of the config file also makes these 2 core options useless probably:

image

gingerbeardman commented 2 years ago

Good points.

I hope that Project IO can get this core back to a useful state, with as many options as possible/sensible migrating to RetroArch menu.

This is my most used core so I am heavily invested in the outcome.

inactive123 commented 2 years ago

OK, after talking with @jdgleaver, while we search for a more standardized solution for this, in the meanwhile I guess I can bring back the external file loading so that people can still use the emulator in a mostly functional state.

one question though to @bslenul and @negativeExponent - can I at least take out the 'saving' of the config file? I.e. can we just make the config file operations in the core read-only, so it can only be loaded in, but it doesn't actually bother tryign to write it back to disk? Becuase as far as I can imagine, there is nothing that has to be written back to this external config file right? or is that a wrong assumption?

bslenul commented 2 years ago

The "Save FDD/HDD Paths" core options would definitely need write access since it saves the paths in the config file. Although these options don't seem to work properly on Windows 🤔

The StartDir thing is read-only however, so it should be fine for this one.

inactive123 commented 2 years ago

OK, I put it back -

https://github.com/libretro/px68k-libretro/commit/ebe9e166918ec30133b3c095c4c7eb5a97aafe99

let me know if it resolves the issues for you people.

bslenul commented 2 years ago

Hey, just tested that last commit on both Windows and my Linux VM, unfortunately that didn't work, it doesn't read StartDir and it doesn't write the FDD paths either.

edit: No error in logs or anything.

inactive123 commented 2 years ago

Fine, I've roleld it all back.

It should work now again.

I will have to redo all rebasing and cleanups I was doing and this time just not touch this 'external config file reading' until we have a better solution.

I'd like to make a humble request if I may. I would like no more interferences after this where people start whining over removed log messages, removed this/that. This entire codebase's upstream has not been active since 2014, the codebase is a disaster, it's chockful of Win32-isms, encoding issues, pointer arithetmic issues, and hard tough decisions need to be made to whip this core into something good. So you guys will have to understand from this point on why this entire codebase needs to be redesigned and re-engineered and we cannot be having all this armchair refereeing and second guessing and nagging over design decisions. We cannot have this codebase remain in this terrible condition, it really is right now plain and simply a bad core that reflects badly on Libretro/RetroArch, and it will need heavy alteration before it can be considered 'good'. And we aren't going to get there by considering anything in this codebase to be a sacred cow that cannot be touched.

gingerbeardman commented 2 years ago

OK, agreed.

My issue was not that it should not be touched, but rather to understand the reasons for the removals (eg. removing the original Japanese comments is removing valuable information about the operation/logic/design of this emulator ...IMHO)