ravignir / 5Hex-Tileset

A MASSIVE tileset mod by Ravignir. New tile graphics, new unit graphics. Compatible with: RekMOD, Ancient and Medieval civs mods. Leave a star on the mod page if you like it.
128 stars 16 forks source link

Lumber Mill not showed #2

Closed AdityaMH closed 3 years ago

AdityaMH commented 3 years ago

I think don't need description. IMG_20210521_011417

ravignir commented 3 years ago

Thank you for feedback, fixed in 341e67652848fa29d33c13f79995d676778b220c

AdityaMH commented 3 years ago

But, me after updated mod. This bug still happen.

ravignir commented 3 years ago

Hmm, looks like Plains and Grasslands are the only tiles that have this issue. This must be connected to unciv code, and i can't fix it with a json.

AdityaMH commented 3 years ago

@SomeTroglodyte , @yairm210 Can you solve this?

ravignir commented 3 years ago

i can solve it with 2 new sprites, but the thing i that's not a real solution to a problem.

SomeTroglodyte commented 3 years ago

Uh, from stepping getTileBaseImageLocationsNew you can either have a json entry for "Plains+Hill+Forest+Lumber mill" or an image named like that or 4 separate images, you can't mix rules... and in my FantasyHex "Lumber mill.png" is missing. Strange the code actually returns "TileSets/FantasyHex/Tiles/Lumber mill" at that point when I have 5Hex perma'ed... Maybe that's replaced later.

ravignir commented 3 years ago

"Plains+Hill+Forest+Lumber mill" does work. Any tile combination with Hills does. "Tundra+Forest+Lumber mill" also works. The only tiles that do not work are "Grassland+Forest+Lumber mill" and "Plains+Forest+Lumber mill", but i am completely clueless why, given there are no issues with Tundra tile.

SomeTroglodyte commented 3 years ago

To be clearer: Priority is

  1. Look into ruleVariants for complete list , in this case "Plains+Hill+Forest+Lumber mill". If found, follow it. Else:
  2. Look for an image with that same name. If it exists, use it. In this case "TileSets/FantasyHex/Tiles/Plains+Hill+Forest+Lumber mill" Else:
  3. Piece the thing together from discrete images, one by one. One Image Plains, one Hill, one Forest, one Lumber Mill.
This replacement with a jota more flexibility: ```kotlin fun getTileBaseImageLocationsNew(viewingCiv: CivilizationInfo?): List { if (viewingCiv == null && !showEntireMap) return listOf(tileSetStrings.hexagon) if (tileInfo.naturalWonder != null) return listOf(tileSetStrings.getTile(tileInfo.naturalWonder!!)) val shouldShowImprovement = tileInfo.improvement != null && UncivGame.Current.settings.showPixelImprovements val shouldShowResource = UncivGame.Current.settings.showPixelImprovements && tileInfo.resource != null && (showEntireMap || viewingCiv == null || tileInfo.hasViewableResource(viewingCiv)) var resourceAndImprovementSequence = sequenceOf() if (shouldShowResource) resourceAndImprovementSequence += sequenceOf(tileInfo.resource) if (shouldShowImprovement) resourceAndImprovementSequence += sequenceOf(tileInfo.improvement) resourceAndImprovementSequence = resourceAndImprovementSequence.filterNotNull() val terrainImages = (sequenceOf(tileInfo.baseTerrain) + tileInfo.terrainFeatures.asSequence()).filterNotNull() val allTogether = (terrainImages + resourceAndImprovementSequence).joinToString("+") val allTogetherLocation = tileSetStrings.getTile(allTogether) val terrainLocation = tileSetStrings.getTile(terrainImages.joinToString("+")) val resourceAndImprovementLocation = tileSetStrings.getTile(resourceAndImprovementSequence.joinToString("+")) return when { tileSetStrings.tileSetConfig.ruleVariants[allTogether] != null -> tileSetStrings.tileSetConfig.ruleVariants[allTogether]!!.map { tileSetStrings.getTile(it) } ImageGetter.imageExists(allTogetherLocation) -> listOf(allTogetherLocation) ImageGetter.imageExists(terrainLocation) && ImageGetter.imageExists(resourceAndImprovementLocation) -> listOf(terrainLocation, resourceAndImprovementLocation) else -> getTerrainImageLocations(terrainImages) + getImprovementAndResourceImages(resourceAndImprovementSequence) } } ```

... wouldn't help because it only additionally looks for "TileSets/FantasyHex/Tiles/Plains+Hill+Forest" plus "TileSets/FantasyHex/Tiles/Lumber mill"...

SomeTroglodyte commented 3 years ago

Tundra+Hill+Forest+Lumber mill works and Plains doesn't because the 4-part image is there and the other 4-part isn't.

I'd say from the above: Create one image each for all base terrains, features, resources, improvs, using transparcency. Stash away all "+" png's. Empty rules. Test. Everything should be there as long as there's enough transparency... What looks bad, do either an extra image for just one specific combo using the long name -or- do a ruleset entry for each specific combo that looks bad, and there you can use file names freely... ??????? My guess. Not my code.

SomeTroglodyte commented 3 years ago

Or, top-down instead of bottom-up, do one nice little "Lumber mill.png" with as much transparency as possible and look how far that gets you.

Idea - should I cheat a map having all possible combinations, including those forbidden by rules but not by class structure?

SomeTroglodyte commented 3 years ago

"Plains+Hill+Forest+Lumber mill" does work. Any tile combination with Hills does. "Tundra+Forest+Lumber mill" also works. The only tiles that do not work are "Grassland+Forest+Lumber mill" and "Plains+Forest+Lumber mill", but i am completely clueless why, given there are no issues with Tundra tile.

Oh - I tested with Plains+Hill+Forest+Lumber mill only, because Plains+Forest+Lumber mill looked OK...? Then again I haven't updated 5Hex for a few days.

Still, observations on how getTileBaseImageLocationsNew ticks should still explain every single result. Except if code I haven't seen sometimes calls another tile look resolving helper... But I don't think so. Then again, I haven't looked into the commit diffs, I've only set a conditional breakpoint in TileGroup for my tile's coords...

ravignir commented 3 years ago
"Plains+Forest+Lumber mill": ["Plains","Forest+Lumber mill"]
"Grassland+Forest+Lumber mil": ["Grassland","Forest+Lumber mill"]
"Tundra+Forest+Lumber mill": ["Tundra","TF+Lumber mill"]

"Plains+Hill+Forest+Lumber mill": ["Plains","Hill","HF+Lumber mill"]
"Grassland+Hill+Forest+Lumber mill": ["Grassland","Hill","HF+Lumber mill"]
"Tundra+Hill+Forest+Lumber mill": ["Tundra","Hill","HF+Lumber mill"]

Screenshot_20210521-183935(1) Makes completely no sense imo. All Hill types are ok, all of tundra is ok, only these bare plains and bare grassland tiles aren't.

ravignir commented 3 years ago

Looks to be working after I've added "Plains+Forest" and "Grassland+Forest" to tileset.json Apparently they are not redundant.

SomeTroglodyte commented 3 years ago

Looks to be working after I've added "Plains+Forest" and "Grassland+Forest" to tileset.json

Should be possible to have it layer Forest onto Plains and Grassland separately, but then it would be the same forest and looks would depend on transparency.

Another thing: ModManager will not delete any files on updating a mod! A fix is somewhere deep in my stashes... Meanwhile, everybody should delete mod first then reinstall...

SomeTroglodyte commented 3 years ago

image I must be doing something wrong myself - that's not even adding lumber mills...

ravignir commented 3 years ago

Btw, that looks like heavily outdated version of 5Hex.

SomeTroglodyte commented 3 years ago

Freshly deleted + downloaded using the mod manager an hour ago?

ravignir commented 3 years ago

Weird. It kind of looks like if it was not reading tileset.json and the grasland+hill+forest tile looks like in the old version. I remember having some similar issues where the game used 5Hex even tho i did not have it in mods folders as it was a freshly compiled jar.

SomeTroglodyte commented 3 years ago

I'm not worried - I hope the little code analysis helped.