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

Missing tiles in debug.log separate from missing with looks_like #39358

Merged
merged 4 commits into from
Apr 11, 2020

Conversation

johnstoecker
Copy link
Contributor

@johnstoecker johnstoecker commented Apr 8, 2020

Summary:

Summary: None

Purpose of change

Currently, when reloading a tileset, debug.log outputs missing tiles, even if their "looks_like" definition is in the tileset.

For example, when the RetroDays tileset is reloaded, debug.log displays "alexandrite_silver_ring" as missing. However, "alexandrite_silver_ring" has copy_from "garnet_silver_ring" which looks_like "silver_ring". The tile_config.json in RetroDays has "silver_ring" defined, so we might want to separately log it.

Cleaning up this output will help tileset maintainers more easily find missing tiles.

Describe the solution

Use the find_tile_looks_like, and if that is defined in the tileset, don't output as missing.

The output would look like:

Missing item: kevlar_silver_ring
Missing item (but looks_like tile exists): alexandrite_silver_ring

Testing

Reloaded a few tilesets, made sure that they worked in game. Checked the output in debug.log for missing tiles. The missing ones are actually missing, and the ones with missing looks_like still appear, but with their generic tile.

@ZhilkinSerg
Copy link
Contributor

Tiles for entities with looks_like are still missing from the tileset and should still be reported. Maybe in !a?a separate line in the report.

@johnstoecker
Copy link
Contributor Author

Ah, I see -- my motivation was originally to update a tileset. Then, when I looked at the number of missing tiles, it seemed very daunting....

Is the intention of "looks_like" to indicate that something should look exactly like something else (i.e. alexandrite_silver_ring vs garnet_silver_ring), or is that something that we should leave up to the tileset maintainers?

@ZhilkinSerg
Copy link
Contributor

looks_like is just a fallback mechanism - it is used when tileset is missing a tile for an object. We are adding it to objects, but that does not necessary means we don't have to add unique tiles for that objects anymore.

@anothersimulacrum
Copy link
Member

Yes, the proper way to say that we want tiles to look the same is to specify them within the tileset definition.

@johnstoecker
Copy link
Contributor Author

Updated to show missing looks_like in a new line. With that in mind, my proposed change might make things worse, and not better -- tile maintainers would be more likely to rely on looks_like, and not the proper way of copying ids inside the tile_config, or creating new tiles for each looks_like.

I'll leave the PR open, but feel to close it.

@johnstoecker johnstoecker changed the title Missing tiles in debug.log no longer ignores looks_like Missing tiles in debug.log separate from missing with looks_like Apr 8, 2020
@ZhilkinSerg
Copy link
Contributor

I don't see how it can make things worse - you've basically just split output to two rows. Number of reported tiles would be the same.

@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` SDL: Tiles / Sound Tiles visual interface and sounds. labels Apr 8, 2020
src/cata_tiles.cpp Outdated Show resolved Hide resolved
src/cata_tiles.cpp Outdated Show resolved Hide resolved
src/cata_tiles.cpp Outdated Show resolved Hide resolved
src/cata_tiles.cpp Outdated Show resolved Hide resolved
@kevingranade kevingranade merged commit 0201326 into CleverRaven:master Apr 11, 2020
fengjixuchui added a commit to fengjixuchui/Cataclysm-DDA that referenced this pull request Apr 11, 2020
Merge pull request CleverRaven#39358 from johnstoecker/missing-tiles-…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` SDL: Tiles / Sound Tiles visual interface and sounds.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants