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

[m3u] NEW Disk Control Interface v1.0 bug reports and discussions #118

Closed negativeExponent closed 3 weeks ago

negativeExponent commented 4 years ago

Now, its easier to see disk labels instead of indexes

Screenshot_2020-01-23_12-43-09 Screenshot_2020-01-23_12-58-12

@jdgleaver

Darknior commented 4 years ago

Excellent it will really help users :D Thanks a lot

jdgleaver commented 4 years ago

@negativeExponent Nice!

This is what I did for the P-UAE core: https://github.com/libretro/libretro-uae/pull/223

It allows custom labels, which I've found to be quite useful. Would it be worth adding something similar here?

negativeExponent commented 4 years ago

but doesn't this break the .m3u specs though by adding | after each line? in anycase maybe you can help out in that area coz im prettry much clueless with strings. ill send the PR in awhile...

jdgleaver commented 4 years ago

but doesn't this break the .m3u specs though by adding | after each line?

Indeed it does, but it's an optional core-specific extension to the format, so I see no harm in it :)

(and | isn't a valid file path character, so you won't get a false match from a file name)

in anycase maybe you can help out in that area coz im prettry much clueless with strings.

Sure, no problem! It will probably be next week, though - I want to add disk control v1 support to the PSX cores and Flycast first, so we get some good coverage of 'popular' systems.

negativeExponent commented 4 years ago

bah, this should be better now...

granada (1990)(wolf team)(disk 1 of 3).dim
granada (1990)(wolf team)(disk 2 of 3).dim
granada (1990)(wolf team)(disk 3 of 3).dim

Screenshot_2020-01-24_02-01-15

Street Fighter II Champion Edition (1993)(SPS)(Disk 1 of 4)(System).dim|SYSTEM
Street Fighter II Champion Edition (1993)(SPS)(Disk 2 of 4)(Disk 1).dim|DISK 1
Street Fighter II Champion Edition (1993)(SPS)(Disk 3 of 4)(Disk 2).dim|DISK 2
Street Fighter II Champion Edition (1993)(SPS)(Disk 4 of 4)(Disk 3).dim|DISK 3

Screenshot_2020-01-24_01-58-04

negativeExponent commented 4 years ago

absolute paths should probably work now, though i dont understand why anyone would need to use one (maybe network path's?)

anyways its just linux tested. it should work with windows as well but who knows, core is tricky in some parts...

Darknior commented 4 years ago

We always use relative path because like this our game set can be use on different systems. My gamelist works fine on Batocera and an old Retropie. I don't use absolute path because the rom directory is mount in two different directories, but and relative path works fine ;) But it's a good idea for some other users ;)

Street Fighter II Champion Edition (1993)(SPS)(Disk 1 of 4)(System).dim|SYSTEM

I don't really understand ? Do you put a "|" after the file type to name the disk in the menu ? Because if i detect and launch ".dim" and when to use your system it will not work for me :( And Batocera users for exemple ...

Why don't you detect "[ ]" in name file and use them ? Like this :

Street Fighter II Champion Edition (1993)(SPS)(Disk 1 of 4)[System].dim

Thanks

negativeExponent commented 4 years ago

@Darknior to keep it short, first, the delimiter "'|" is based on specs of the new disk control interface, so that should be followed 2nd your example suggests having to rename the files. aside from that we have to do a lot more checking on the strings than needed.

the delimiter | is not yet merged, hence it probably fails. im waiting for buildbot to clear up before i can merge it. if you are able to compile, you can test out the pending PR.

thanks for testing and feedback.

Darknior commented 4 years ago

It will compiled if the future Batocera build, almost all the 15 days lol I understand your disk control interface can read it, for sure. I speak about the external launcher that launch games in Batocera, Retroarch, Recallbox, Launchbox, etc ... They don't know this file type and will not show and launch it. We will not use this option, impossible for us.

Rename games ... yes lol for my part all are renamed from a long time i don't like the original names. But maybe i made a mistake to do it ...

thanks for your answer

jdgleaver commented 4 years ago

@negativeExponent Very nice!

Just a couple of things:

disk.dci_version = 0;
environ_cb(RETRO_ENVIRONMENT_GET_DISK_CONTROL_INTERFACE_VERSION, &disk.dci_version);
negativeExponent commented 4 years ago
disk.dci_version = 0;
environ_cb(RETRO_ENVIRONMENT_GET_DISK_CONTROL_INTERFACE_VERSION, &disk.dci_version);

there is already pending PR to correct this, im just waiting for a go signal to merge it since buildbot is bz with other stuff

jdgleaver commented 4 years ago

Oh right! Sorry, I just checked out a copy of master and grepped. I should have looked at the PRs as well. :)

Good work!

bslenul commented 4 years ago

Hey!

Whenever I load a .m3u file I get a "Failed to set last used disc..." notification, but the game actually works perfectly fine as far as I can tell.

My m3u:

Akumajou Dracula (1993)(Konami)(Disk 1 of 2).dim|Disk 1
Akumajou Dracula (1993)(Konami)(Disk 2 of 2).dim|Disk 2

Also tried without the |Disk X part (really love that feature btw), same result. And content of the .ldci:

{
  "version": "1.0",
  "image_index": 0,
  "image_path": ""
}

I tried deleting it so it recreates a proper new one just in case but nope, path stays empty. If I switch disks through the Disc Control menu the file still looks like that, even the index is not being updated.

Tested on both Windows 10 and a Linux Mint VM with latest RA and core nightlies.

edit: Hope this is fine using this thread, I can create a new issue if you guys prefer.

jdgleaver commented 4 years ago

Ah, I think I can see why that happens - the core assigns a set_initial_image() function, so the frontend thinks this functionality is supported - but it's actually not supported at all in the underlying code (and shouldn't be, since floppy-based systems always start from the first disk).

If this line:

https://github.com/libretro/px68k-libretro/blob/415dd30665f92d86a08630d2a25d58b89c60bc80/libretro.c#L293

is changed to:

dskcb_ext.set_initial_image = NULL;

...it should get rid of the notification.

bslenul commented 4 years ago

Yup that worked, no notification and no more .ldci file being created! Thanks 👍

It was harmless but I saw a couple people being confused by it, thinking they did something wrong :p

jdgleaver commented 4 years ago

Thanks for testing :)

@negativeExponent Would you like to push the above change? Or I can make a PR if you prefer.

negativeExponent commented 4 years ago

sorry late reply. bad internet. merged the changes. thanks for reporting and suggesting fix.

jdgleaver commented 4 years ago

@negativeExponent Thanks very much for merging the change! :heart:

Darknior commented 3 years ago

Now it works perfectly for me Why no one close this issue ?

negativeExponent commented 3 weeks ago

it appears that there has been no reported issue about this anymore. so ill close it.