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

Changing tileset on the main menu moans about layering ids not existing #78592

Open
Procyonae opened this issue Dec 15, 2024 · 3 comments · May be fixed by #78670
Open

Changing tileset on the main menu moans about layering ids not existing #78592

Procyonae opened this issue Dec 15, 2024 · 3 comments · May be fixed by #78670
Labels
(S2 - Confirmed) Bug that's been confirmed to exist

Comments

@Procyonae
Copy link
Contributor

Procyonae commented Dec 15, 2024

Describe the bug

When you change tileset the newish layering stuff verifies ids except on the menu only /core has been loaded so it errors that the ids are invalid

Attach save file

N/A

Steps to reproduce

Change tileset/overmap tileset while on the menu with at least one ending as a tileset with custom layering specified (eg Ultice/MSX)
Get errors

Expected behavior

No silly errors whether that's by not verifying the ids if /data/json isn't loaded or just not verifying them period bc we don't verify any other tileset sprite targets afaik (which would cause errors whenever we obsolete/migrate any id Edit: And wouldn't work period bc then you can't have modded stuff in tilesets unless you also specify the src in the tileset or something)

Screenshots

Example error

 DEBUG    : Layering data: f_flagpole not a valid furniture/terrain object

 FUNCTION : void tileset_cache::loader::load_layers(const JsonObject&)
 FILE     : src/cata_tiles.cpp
 LINE     : 890
 VERSION  : 47f98df320-dirty

Versions and configuration

  • OS: Windows
    • OS Version: 10.0.19045.5247 (22H2)
  • Game Version: 47f98df-dirty [64-bit]
  • Graphics Version: Tiles
  • Game Language: System language []
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth]
    ]

Additional context

Notably we seem to reload or at least reverify the regular tileset when you change overmap tileset (incl when it wasn't shared) which seems incredibly inefficient

@Procyonae Procyonae added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Dec 15, 2024
@RenechCDDA
Copy link
Member

Oh yeah, got this earlier. Confirmed then.

@RenechCDDA RenechCDDA added (S2 - Confirmed) Bug that's been confirmed to exist and removed (S1 - Need confirmation) Report waiting on confirmation of reproducibility labels Dec 16, 2024
@ShnitzelX2
Copy link
Contributor

Yeah, that makes sense, I didn't account for changing tilesets in the title menu. I think that

not verifying the ids if /data/json isn't loaded

is probably the solution, so I'll try to at least patch this soon.

Though, I don't think that switching tilesets in the title menu should functionally do anything but change what's loaded (instead of doing loading immediately after switching), so maybe I'll take a deeper look.

@Procyonae
Copy link
Contributor Author

Outside of loading screens there's no real reason for tilesets to be loaded on the menu at all, making them only load what's necessary for the active mods would be pretty neat if that's doable ye

@ShnitzelX2 ShnitzelX2 linked a pull request Dec 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants