Pidgeot / python-lnp

Cross-platform re-implementation of the Lazy Newb Pack launcher.
ISC License
64 stars 10 forks source link

Missing "installed_raws.txt" causes install failures when graphics dirname changes #149

Closed Pidgeot closed 4 years ago

Pidgeot commented 4 years ago

From @fricy

Minor bug report, mainly for PerX: The 'installed_raws' text genererates the installed tileset/pack configuration from the directory name, and not from the manifest name, and only accepts directories with the exact same filename. This will cause silent save update failures, like when you update Gemset_1.3 to Gemset_1.4. Some flexibility'd be good in the future. For now my advice is to stick to the directory name, and not to use version numbers in folders when we restart the release cycle for df4?.01.

This is pretty close to the current intended behaviour, but clearly has some downsides. The workaround above will work for now, but a fix would be good eventually - possible one or both of the following.

[Issue created by PeridexisErrant: 2015-11-30] [Last updated on bitbucket: 2016-08-10]

[Comment created by jecowa: 2016-08-10]

What happens to graphics packs without a manifest? (they still exist!)

Maybe graphics packs without a title or ID in their manifests could use the full folder name to identify them. Currently it's only using the first word of the folder name and won't install those graphics packs into save files unless the graphics pack has no spaces in its folder name.

If you're using a graphics pack that alters object raws in a way it shouldn't, this might break someone's save.

Is there currently a system to protect save files from graphics packs with outdated raws?

The real answer is to fix that problem (issue #74), but nobody's done it yet because robust and sophisticated handling is hard.

Yeah, that looks complicated.

[Comment created by PeridexisErrant: 2016-08-09] We used to do that, but people naming their graphics pack things like "Gemset 1.4" meant that when end-users upgraded to a new pack (with, eg, "Gemset 1.5") old saves couldn't have their graphics changed - because the old pack wasn't available.

That was arguably correct, but changed because it caused pointless problems for many users.

[Comment created by PeridexisErrant: 2016-08-10] What happens to graphics packs without a manifest? (they still exist!)

If you're using a graphics pack that alters object raws in a way it shouldn't, this might break someone's save. The real answer is to fix that problem (issue #74), but nobody's done it yet because robust and sophisticated handling is hard.

[Comment created by jecowa: 2016-08-09] What if PyLNP uses the graphics pack's folder name as the unique identifier instead of the first word of the graphics pack's manifest title? Folder names have to be unique. I don't think doing it this way should change any of the identifiers in Peridexis' pack.

If it's bad to use folder names, maybe the unique ID could get it's own line in the graphics pack manifest file.

[Comment created by jecowa: 2016-08-09] Instead of having it so that the folder name has to start with the first world of the title, what if the folder name has to start with a new unique name line?

Example Manifest.json:

#!json

{
    "author": "Jimbo",
    "content_version": "v1.23",
    "df_min_version": "0.42.04",
    "df_max_version": "0.43.05",
    "title": "ASCII Land 14x20",
    "uniqueID": "ASCII Land,
    "tooltip": "unique blend of ASCII and graphics."
}

valid folder names:

Just like how PyLNP currently is doing it, but using a new "unique ID" instead of the first word of the graphics pack's title. Maybe calling it "folder name" instead would make its function more clear to users.


Here's another thought: Does PyLNP really need to know what graphics pack is in a save file in order to change it? Can't it just dump out the raws, refill from the specified baselines, and then apply the new graphics pack? Why does it need to know what graphics pack is currently installed?

[Comment created by Pidgeot: 2016-03-14] Removing milestone: 0.11 (automated comment)

[Comment created by Pidgeot: 2016-01-21] Make cross-version graphics merging more flexible.

Closes issue #97.

→ <>

[Comment created by PeridexisErrant: 2016-01-19] Re: dropping installed_raws.txt - we may want to do this at some point, but I don't think there's a compelling reason to do it now.

Re: additional fields in manifest - I'd like to keep it as simple as possible, and tile dimensions are something we can calculate from the actual images if we need to know (for, eg, suggested optimal tile size). If the tilesize should go in the tooltip, just write it in manually! Allowing extra information in the title is why I propose the split-on-space fallback matcher, so that would also work manually.

[Comment created by fricy: 2016-01-18] I think tilesize can be removed from the pack name, and instead a new value for size_x, size_y added to the manifest. The launcher could use that, and maybe even the pack version number to generate a pack description/name in the list. Or a tooltip. Whatever you are comfortable with. Maybe even use this information in the Advanced/resolution panel: query the screen resolution (if possible), and calculate a best guess tile number.

Another suggestion is to drop the installed_raws.txt in favor of a json config that'd store the same information but in a better structured, easier to parse format. Of course a fallback converter needs to be added for backwards compatibility. I'm thinking something like this:

{
"save_version": "0.42.04", --never change this field!
"baseline": "vanilla", --always vanilla/ascii/default for now, but let's add this value in case we need it later for Masterwork or something
"graphics_name": "Phoebus", --title
"graphics_url": "https://github.com/DFgraphics/Phoebus/releases", --can be usefull for updating
"graphics_version": "0.42.04", --df_max_version, can be usefull in the future  
"graphics_x": "16",
"graphics_y": "16",
"mod_1_name": "Modname",
"mod_1_url": "http://",
"mod_1_custom1": "Whatever setting 1",
"mod_1_custom2": "Whatever setting 2",
"mod_2_name": "Modname",
"mod_2_url": "http://",
"mod_2_custom1": "Whatever setting 1",
"mod_2_custom2": "Whatever setting 2"

}

[Comment created by Pidgeot: 2015-11-30] Having the manifest provide a "canonical" name for merging seems quite reasonable to me. If missing, use the folder name as the default value.

However, there is the possibility that two packs might have the same canonical name this way. Some sort of conflict resolution would probably need to be implemented. Could just be a question of generalizing the current DF selection dialog to show arbitrary content...

[Comment created by PeridexisErrant: 2016-01-20] Looks like 61a4445 fixed the problem with a missing install log - that is, such saves are now explicitly rather than silently skipped. I'll add a warning log entry too.

[Comment created by PeridexisErrant: 2016-01-18] @fricy Several graphics packs include details such as tile size etc, which make sense as a title but might change without breaking raw-compatibility.

I think the best solution is to use fallbacks (and logging) - first try to match exact dirname, then exact title, then first part of title (splitting on whitespace).

Where an installed_raws.txt but no mods are present, we only need DF-version compatible raws and can go without the old graphics set.

In the normal, well-configured case this will maintain current behaviour. If the dirname changes, a good manifest means it's still OK. If the manifest is iffy (eg because of a fork), it might still work.

Key point - we want to warn-and-fail on a missing install log, but handle updated graphics packs as well as possible; these are mostly-separate problems.

[Comment created by PeridexisErrant: 2015-12-13] ... I duplicated this in issue #102, now closed.

Currently, PyLNP will not update graphics on saves without raw/installed_raws.txt present.

This was added to prevent accidental breaking of saves if mods or non-standard raws are present. However, it also means that PyLNP does not handle graphics on saves started under vanilla DF or other packs, or by PyLNP before the first time a graphics pack was installed.

  • The popup messages should distinguish between "no saves were found" and "x saves were found without raw/installed_raws.txt, and will not be updated". Currently only the former is implemented.

  • If we move to using BAMM or PyDwarf or the current DFRaw module, it should be possible to detect graphics-only changes, including whether raws are vanilla-except-graphics. In this case we could update a save even without the log present.

  • Eventually we might also use a parser rather than a line-diff to update graphics (and mods?), which would improve confidence in the output enough to discard the requirement for that file.

[Comment created by PeridexisErrant: 2015-12-13] Issue was duplicated due to affecting two pack maintainers, which makes it a priority. I'll try to get to this eventually, but if anyone else wants a go feel free.