[Issue] 1G1R sorting sets all missing ROMs to ignored

TylerVigario commented 1 year ago

Using sort-roms in 1G1R mode sets all missing ROMs to ignored. For example, I recently imported a 3DS P/C DAT and before importing any ROMs I ran sort-roms and it set all releases from the DAT as ignored.

Screenshot

alucryd commented 1 year ago

Did you set any region in REGIONS_ALL or REGIONS_ONE? By default it should keep everything if they're empty, although I don't use that mode it is verified by a unit test.

Can you paste the output of oxyromon config -l?

TylerVigario commented 1 year ago

Of course, here you go.

DISCARD_FLAGS = Aftermarket,Debug,Virtual Console,Atari Anthology,A
lt,Audio Tapes,Activision Anthology - Remix Edition,Unl,BIOS,Homebr
ew,Test Program,Wii Virtual Console,GameCube,Wi-Fi Kiosk,Castlevani
a Anniversary Collection,e-Reader Edition,GameCube Edition,Contra A
nniversary Collection,Collection of Mana,Collection of SaGa,Possibl
e Proto,Disney Classic Games,Pirate,Program,Castlevania Advance Col
lection,Debug Version,Kiosk,Namcot Collection,Wii U Virtual Console
,Switch Online,Enhancement Chip,Wii,Mega Man X Legacy Collection,Ar
cade,Doritos Promo,Darius Cozmic Collection,Tech Demo,Auto Demo,Seg
a Ages,SEGA Classic Collection,Sega Channel,Genesis Mini,Sega Smash
 Pack 2,PC Rerelease,Sega Smash Pack,Sonic Classic Collection,Sonic
 Mega Collection,Bonus Disc
DISCARD_RELEASES = Beta,Proto,Sample,Demo,Hack,Bootleg,Program
HASH_ALGORITHM = CRC
REGIONS_ALL =
REGIONS_ONE = US,ZZ
ROM_DIRECTORY = /mnt/storage/__groupfolders/1/Roms
TMP_DIRECTORY = /tmp

alucryd commented 1 year ago

Thanks! Hmm, with that configuration it should at least keep US roms, I have a unit test covering just that, weird. I'll try to replicate over the week. Are you passing any flag to sort-roms?

Note that right now ZZ only works in REGIONS_ALL on the develop branch, I had considered having a fallback in 1G1R as well, but there is no way to objectively sort regions so ZZ didn't make much sense in REGIONS_ONE as the end result would just be to keep the first game in the array (which would be the first game name alphabetically).

What do you expect from having ZZ in REGIONS_ONE, something like having Japanese only releases in the 1G1R directory? In most cases (if not all) it should be covered by explicitly listing the desired regions in order, like US,JP for instance.

Edit: Glad to see I'm not the only one with an extensive DISCARD_FLAGS list :)

TylerVigario commented 1 year ago

Nothing passed to sort-roms.

As for ZZ my goal was to catch World releases but this is probably a misunderstanding on my part. I am still trying to wrap my head around how you are matching releases from different naming conventions.

alucryd commented 1 year ago

I'm using shiratsu naming under the hood: https://docs.rs/shiratsu-naming/latest/shiratsu_naming/region/enum.Region.html

By default, World is expanded to US, EU and JP in shiratsu, so if you have at least one of the 3 in REGIONS_ONE, it will match.

TylerVigario commented 1 year ago

Perfect! I haven't tried out Rust yet but this project might inspire me to try it. Thanks again!

alucryd commented 1 year ago

I was able to reproduce, which makes me wonder why that particular unit test is passing. Will have to debug that one.

alucryd commented 1 year ago

@TylerVigario Oh! I found what leads to this! I made sure that there would always be at least one game chosen as 1G1R, say you have US,EU as priorities, and you don't have the US clone, if you do have the EU clone it will end up in the 1G1R directory. So one of my criteria to be 1G1R is that you actually have the rom, hence when you have none, and haven't set anything in ALL_REGIONS, everything ends up as ignored. My test didn't catch that because I import roms as part of the test. And I never caught onto that myself as I've always had sth in ALL_REGIONS.

Will need to devise some way to change the logic without breaking my original intent :/

TylerVigario commented 1 year ago

@alucryd Keeping the best present ROM is a great 1G1R strategy (I came from Retool that used a "best overall ROM" method).

I would catch the case that no parent or clone ROM currently exists. We will probably need to calculate best present candidate (current logic) as well as best possible ROM. This may require another list (best overall vs best candidate) for tracking.

To improve your testing, I would check for a desired release of a ROM that isn't yet imported, checking if it's wanted or ignored.

EDIT: Reworded for better clarity

EDIT (1/10/23): WOW just barely seen you commited an update! Testing it out now.

alucryd commented 1 year ago

Yes! I had not commented about it yet as there's a second part coming that will take the form of another config option. I also came from other tools which didn't care whether you had the ROM or not for 1G1R, which is why I made it so initially in oxyromon. However I realize that other tools' behavior might also be desired and is a valid use case.

This config will probably be named REGIONS_ONE_STRICT and default to false, this will be the current behavior where the best available ROM is elected as 1G1R. Setting it to true will mimic other tools where the ROM being available or not is not a factor in the election process.

Thank you so much for the donation BTW, it means a lot!

alucryd commented 1 year ago

There you go: https://github.com/alucryd/oxyromon/commit/c5b50e3238f3011feb9dd9d207455bd041567138

Implementing that setting was quicker than I thought. Please let me know how that goes.

TylerVigario commented 1 year ago

This is awesome news! Unfortunately, it's trashing what it clearly the best ROM in certain circumstances. Here is just one example of multiple ROMs it's trying to get rid of in my collection - screenshot

"Extreme Skate Adventure (USA, Europe).7z" -> "/mnt/storage/__groupfolders/1/Roms/Nintendo - Game Boy Advance/1G1R/Extreme Skate Adventure (USA, Europe).7z"
"Extreme Skate Adventure (Europe) (Fr,De).7z" -> "/mnt/storage/__groupfolders/1/Roms/Nintendo - Game Boy Advance/Trash/Extreme Skate Adventure (Europe) (Fr,De).7z"
"Extreme Skate Adventure (USA) (Rev 1).7z" -> "/mnt/storage/__groupfolders/1/Roms/Nintendo - Game Boy Advance/Trash/Extreme Skate Adventure (USA) (Rev 1).7z"

Figured out it want's to keep the original (USA, Europe) ROM and not the newer (USA) (Rev 1) release.

Thank you so much for the donation BTW, it means a lot!

No problem, it's the least I could do!

EDIT: Updated

alucryd commented 1 year ago

Hmm, thanks for the heads up, I only relied on unit tests for that one, will give it a go in the real world. I think it's more of a sorting order here, and not what I just implemented.

Edit: Okay, the rev 1 is actually a clone in the DAT, I'm guessing the others are too, so it makes sense with the current code that the original would be chosen as parents take precedence and it also contains your selected region. However the rev 1 is indeed newer, so I will just forget the whole idea of parent/child and just consider them as a group of games with no predetermined hierarchy.

alucryd commented 1 year ago

@TylerVigario There you go: https://github.com/alucryd/oxyromon/commit/8cd06de1c5528a37a8e6ab52134c4b7736a790f0

There was indeed a sorting error when revisions weren't the parent roms in DATs. I completely changed the sorting algorithm, now all games in a group are thrown in a pool and the parent is not automatically winning if one of its regions match.

The new sorting weights both the revision, and the parent/clone relationship. Being higher rev weighs twice as much as being the parent.

Applying it to my entire collection showed no obvious regression, and a few revs have been promoted to 1g1r, including your example from above:

Please select systems: Nintendo - Game Boy Advance
Processing "Nintendo - Game Boy Advance"
Summary:
"Extreme Skate Adventure (USA) (Rev 1).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/1G1R/Extreme Skate Adventure (USA) (Rev 1).7z"
"Sims, The - Bustin\' Out (USA) (En,Fr,De,Es,It,Nl) (Rev 1).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/1G1R/Sims, The - Bustin' Out (USA) (En,Fr,De,Es,It,Nl) (Rev 1).7z"
"Extreme Skate Adventure (USA, Europe).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/Extreme Skate Adventure (USA, Europe).7z"
"Sims, The - Bustin\' Out (USA, Europe) (En,Fr,De,Es,It,Nl).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/Sims, The - Bustin' Out (USA, Europe) (En,Fr,De,Es,It,Nl).7z"

TylerVigario commented 1 year ago

@alucryd This is awesome news and works great!

I have a couple edge cases for you:

(Rumble Version) should have higher weight (for Game Boy Color)
ROMs can be clones of (Rev X) games but don't include (Rev X) in their own titles. This breaks the current logic. To fix this, we need to consider clones of (Rev X) games to be newer than there parent. See issue examples below.

Examples: <game name="Tonka Raceway (USA)" cloneof="Tonka Raceway (Europe)"> <game name="Tonka Raceway (USA) (Rumble Version)" cloneof="Tonka Raceway (Europe)"> "Tonka Raceway (USA) (Rumble Version).7z" -> "/mnt/storage/__groupfolders/1/Roms/Nintendo - Game Boy Color/Trash/Tonka Raceway (USA) (Rumble Version).7z"

<game name="Wild Gunman (Japan, USA) (En)" cloneof="Wild Gunman (World) (Rev 1)"> "Wild Gunman (Japan, USA) (En).7z" -> "/mnt/storage/__groupfolders/1/Roms/Nintendo - Nintendo Entertainment System/Trash/Wild Gunman (Japan, USA) (En).7z"

<game name="Daimakaimura ~ Ghouls'n Ghosts (Japan, USA) (En)" cloneof="Daimakaimura ~ Ghouls'n Ghosts (World) (Rev A)"> "Daimakaimura ~ Ghouls\'n Ghosts (Japan, USA) (En).7z" -> "/mnt/storage/__groupfolders/1/Roms/Sega - Mega Drive - Genesis/Trash/Daimakaimura ~ Ghouls'n Ghosts (Japan, USA) (En).7z"

alucryd commented 1 year ago

@TylerVigario I think I'll add another config setting, similar to DISCARD_FLAGS but doing the opposite, something like PREFER_FLAGS. This will add weight to specific flags, like Rumble Version (provided this is parsed as a flag, need to double check).

Not sure I follow your 2 rev examples though, it looks like the current logic did work since the non rev versions are moved to the trash directory, which means oxyromon has elected the newer world revisions.

TylerVigario commented 1 year ago

@TylerVigario I think I'll add another config setting, similar to DISCARD_FLAGS but doing the opposite, something like PREFER_FLAGS. This will add weight to specific flags, like Rumble Version (provided this is parsed as a flag, need to double check).

This is a great idea!

Not sure I follow your 2 rev examples though, it looks like the current logic did work since the non rev versions are moved to the trash directory, which means oxyromon has elected the newer world revisions.

That is only taking into account the file name but you can see from the DAT that they are cloned from this (Rev X) ROM thus making them actually newer. Since they don't include the revision tag in there name as well they are not considered as newer by Oxyromon.

EDIT: Updated

alucryd commented 1 year ago

Ok, I get it. I'm afraid this case and the one we had earlier are mutually exclusive, I can't give being a parent and being a rev the same weight or most elections would end up in a tie, so one side must weigh more. Right now being a rev weighs more, so revs are elected. The earlier logic was equivalent to the parent weighing more, if I go back that way you'll end up with Extreme Skate Adventure (USA) (Rev 1) in the trash directory again because the rev is a clone instead of being a parent (which is arguably probably wrong and should be fixed at the DAT level).

TylerVigario commented 1 year ago

A temporary (hack-ish) solution is to preprocess all similar ROMs, temporarily appending (Rev X+1) to clone titles where their parents include it just for the election process.

However this is not ideal. If I get some time today I'll see what I can come up with for revision weighting.

alucryd commented 1 year ago

Settings added: https://github.com/alucryd/oxyromon/commit/dc43c28162cdb221eea484cbcf2c140213ddcf08

Works fine on your example:

Processing "Nintendo - Game Boy Color"
Summary:
"Tonka Raceway (USA) (Rumble Version).7z" -> "/home/public/Emulation/Nintendo - Game Boy Color/1G1R/Tonka Raceway (USA) (Rumble Version).7z"
"Tonka Raceway (USA).7z" -> "/home/public/Emulation/Nintendo - Game Boy Color/Tonka Raceway (USA).7z"

As for the other matter at hand, my take is hacks are never a good thing, and we should try to get the DAT fixed instead. There are 2 ways to go about it, either we trust that the DAT provider has done their homework, and give parent a higher weight, or we don't and we give revisions higher weight.

I'm trying to find information on the wild gunman revisions, but I can't find any definite answer. It's quite possible that the world rev 1 is newer than the us release, in which case the latter weights would be preferred, but the reverse could be true as well, in which case we should use the former. It is also very possible that the combined USA and Europe release of Extreme Skate Adventure is indeed newer than the USA revision 1, in which case the former weights should be preferred, and again in reverse.

Writing that, I'm leaning towards the former as I would rather trust the DAT provider to correctly pre-sort ROMs, it is their job after all and I'd rather not second-guess it. In case of a mistake it should simply be reported so that it can be fixed in the DAT itself. I think I will revert the weight and give higher precedence back to parents.

TylerVigario commented 1 year ago

I don't believe this is an issue with the DAT. They sort revisions by explicitly flagging successive releases with revisions. In a P/C DAT it becomes a bit more complicated because we have relationships where clones inherit their parents revision. And since clones are not a successive release of the same title they do not merit the revision flag.

alucryd commented 1 year ago

Indeed, that's what I was concluding, hence oxyromon should not bypass that parent/clone information and I should reverse the weights, 2 for parent and 1 for revision.

Take Ocarina of Time for example, the PAL rev 1 is correctly listed as parent, even though the US got a rev 2, the PAL rev 1 is newer. It's not an issue here because NTSC and PAL are incompatible, but on a universal console like handhelds, prioritizing revisions would incorrectly elect the US rev 2, even though the potentially US/EU rev 1 would be newer.

Applied to Extreme Skate Adventure, it's probably a similar case, the common us/eu release is actually newer than the us rev 1, so it should correctly be elected.

https://github.com/alucryd/oxyromon/commit/77cba05a96759c3f2f4f29d6b653b1e14049f5c3

As an added bonus, I found that I had a faulty unit test, and 2 others were effectively useless with the new sorting algorithm, made them useful again.

TylerVigario commented 1 year ago

Indeed, that's what I was concluding, hence oxyromon should not bypass that parent/clone information and I should reverse the weights, 2 for parent and 1 for revision.

Apologies, my misunderstanding.

https://github.com/alucryd/oxyromon/commit/77cba05a96759c3f2f4f29d6b653b1e14049f5c3

Great news! Just got home, will test shortly.

EDIT: This now has the previous issue of favoring the parent. I think there should be no favoritism towards either clone or parent.

This is how I see revision election working:

1 if no revision listed
2 if Rev 1|A
3 if Rev 2|B
If a ROM has a parent, it should start with its parents calculated revision + 1

Example: <game name="Daimakaimura ~ Ghouls'n Ghosts (World) (Rev A)"> = Base + Rev 1|A = 2 <game name="Daimakaimura (Japan)" cloneof="Daimakaimura ~ Ghouls'n Ghosts (World) (Rev A)"> = No region match, disregard ROM <game name="Daimakaimura ~ Ghouls'n Ghosts (Japan, USA) (En)" cloneof="Daimakaimura ~ Ghouls'n Ghosts (World) (Rev A)"> = Parent + 1 = 3

alucryd commented 1 year ago

That's not an issue in my opinion, rather a choice, see explanation above. I don't think there is a right answer at all anyway, since we don't know for sure what DAT producers take into account when selecting the parent, it should in theory be the best version available, then again you could always argue otherwise. Other tools don't necessarily have a very clear process either.

Clones inheriting the parent's revision is wrong IMHO, at the very least not as simple and clear-cut as in your example, here are a few reasons for that:

a (World) rom isn't the same as a (Japan, USA) rom, it is not necessarily a revision nor an upgrade, it's just a different version of the same game, and if it's listed as parent, it's usually newer than its clones, so clones would end up unnecessarily high in the chain even though they're actually older
using your logic, given a rev 1 parent, the rev 0 of the same rom would also inherit the rev 1 since it's a clone, so both would be a tie

The best I can do is give you all the knobs to tweak the sorting however you like, but you must realize that there are mutually exclusive cases like those we've seen in this discussion, and you will probably have to compromise somewhere, although I have a feeling that my proposal below could help you achieve the result you want. BTW newer revision doesn't imply better either, as a matter of fact I can even see use cases where earlier revisions could be preferred (cough Ocarina of Time cough).

By default I will keep prioritizing parents, that way we stay consistent with prior versions of oxyromon. I propose the following knobs:

PREFER_PARENTS: bool, default true
PREFER_VERSIONS: enum (new, old, none), default none
PREFER_REGIONS: enum (broad, narrow, none), default none
PREFER_FLAGS: list, default none

Pretty self-explanatory, except PREFER_REGIONS maybe, broad will prioritize roms targeting multiple regions (like world), narrow will prioritize roms targeting your particular region.

I'll see if I also need to make weights customizable, but I intend to start with equal weights for all 4 and see how that goes.

alucryd commented 1 year ago

There you go: https://github.com/alucryd/oxyromon/commit/dd3caa8db47bef3a1efa192d354b79fa228460bc

Defaulted PREFER_VERSIONS to new after all, that's also what oxyromon did before, only sligthly different with the new logic.

Here's what it changed in the GBA department, with weights of 1 there are some ties, which result in an implicit alphabetical sorting since that's how I retrieve games from the database. That also happens to be what you want to see :) You can also experiment with narrow regions, this will further ascertain the result below, but will also have other consequences you might like (or not).

Please select systems: Nintendo - Game Boy Advance
Processing "Nintendo - Game Boy Advance"
Summary:
"Extreme Skate Adventure (USA) (Rev 1).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/1G1R/Extreme Skate Adventure (USA) (Rev 1).7z"
"Sims, The - Bustin\' Out (USA) (En,Fr,De,Es,It,Nl) (Rev 1).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/1G1R/Sims, The - Bustin' Out (USA) (En,Fr,De,Es,It,Nl) (Rev 1).7z"
"Extreme Skate Adventure (USA, Europe).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/Extreme Skate Adventure (USA, Europe).7z"
"Sims, The - Bustin\' Out (USA, Europe) (En,Fr,De,Es,It,Nl).7z" -> "/home/public/Emulation/Nintendo - Game Boy Advance/Sims, The - Bustin' Out (USA, Europe) (En,Fr,De,Es,It,Nl).7z"
⠙                                                                                                                                                                                                                                                                                                                                                                                                                                          Proceed? [Y/n]

Using broad regions, I saw one example of a rom with only japanese language being selected, it was released in the US, Japan and Europe with japanese language for some reason, might add yet another setting to filter languages, on top of regions if necessary.

I personally think I'll keep the default settings here as it produces reasonable results, although I might still introduce variable weights in the future.

TylerVigario commented 1 year ago

@alucryd Interesting, I have never confronted the option that parents are the best/newest. By definition I assumed they derive clones in creation from their parent. However, any information I found seems vague and built on trust (lol), and furthermore points towards what you've state to be true. Which makes sense, however there are no guarantees towards our goal (of actual newest) based on this information.

I'd like to note, that my intention is to have a similar output to Retool. However, it's become increasingly apparent that this discussion has spilled beyond the original issue. I'll end my discussion here for now and should anything new (evidence-based of course) appear I will bring it up in a separate issue.

As for the recent commits, this is great news! PREFER_PARENTS is a welcome addition, however now I am not so sure I should disable it lol. Going to do some further research and experiment with the settings and see what I find.

alucryd commented 1 year ago

Retool looks really cool, might even use it as a prefilter here if it does correct the parent/clone groups with its clonelists, but I'm guessing it also completely overwrites the original parent which is not something I'd want. Anyway, I'm sufficiently happy with the current results, because I keep all roms from my preferred regions and only use 1G1R as a way to not dump the whole catalog on my various devices I'm always free to reorganize at a later point.

alucryd / oxyromon

[Issue] 1G1R sorting sets all missing ROMs to ignored #60