Request for comments: Prepare resources for better mod-ability #4146
Conversation
SomeTroglodyte
changed the title
|
P.S.: This would cover @GoldenBronze's "River" post cheaply - but as I said, that part is just a demo of the possibilities |
|
If anything, resource uniques should be merged into global uniques, so they won't need any special attention. This is actually much simpler that other solutions as well :) |
| @@ -433,7 +435,7 @@ class CityStats { | |||
| if (cityInfo.getRuleset().modOptions.uniques.contains("Can convert gold to science with sliders")) { | |||
| val amountConverted = (newFinalStatList.values.sumByDouble { it.gold.toDouble() } | |||
| * cityInfo.civInfo.tech.goldPercentConvertedToScience).toInt().toFloat() | |||
| if (amountConverted > 0) // Don't want you converting negative gold to negative science yaknow | |||
There was a problem hiding this comment.
Didn't I just remove the "yaknow" which triggers the spellchecker? ... yup. Such language may be nice to lighten a mood - but aren't we in the worst possible mood beyond redemption when coding anyway? 😉
| @@ -661,8 +661,7 @@ open class TileInfo { | |||
|
|
|||
| if (resource != null && !ruleset.tileResources.containsKey(resource)) resource = null | |||
| if (resource != null) { | |||
| val resourceObject = ruleset.tileResources[resource]!! | |||
| if (resourceObject.terrainsCanBeFoundOn.none { it == baseTerrain || terrainFeatures.contains(it) }) | |||
| if (ruleset.tileResources[resource]!!.mustRemoveFromTile(this)) | |||
There was a problem hiding this comment.
Why not just !isAllowedOnTile(this)? Why make an extra function? I think the performance difference is negligible
There was a problem hiding this comment.
There's a comment to that (is there)? The original code did already check differently, so I left the discrepancy but wrapped it.
There was a problem hiding this comment.
I think this should be the same check, if we're unifying things
| for (terrain in resource.terrainsCanBeFoundOn) | ||
| if (!terrains.containsKey(terrain)) | ||
| lines += "${resource.name} can be found on terrain $terrain which does not exist!" | ||
| warningCount += resource.checkModLinks(this, lines) |
There was a problem hiding this comment.
I really don't think this is good. Mod links should be checked in the ruleset, not in the resource...
There was a problem hiding this comment.
Marginal decision either way. Makes property access a bit shorter on average was the idea methinks.
| * @param ruleset The ruleset to search for viable terrains | ||
| * @return Terrain name or null if impossible due to [ruleset] inconsistency | ||
| */ | ||
| fun getSampleTerrain(ruleset: Ruleset): String? { |
There was a problem hiding this comment.
For one usage? I don't think this is needed.
There was a problem hiding this comment.
I know. That's got to do with "general structure" - I generalized as much as possible first, planning to fold such stuff later if rarely (or not at all) used...
Limit them to civ-global effects? Marble isn't global, or is it? Pretty sure it's per city. And some mod ideas are tile-specific properties, like 'resource allowed on river tiles only' or 'your kumquat disappears if you chop the jungle'... |
|
Marble is global. Marble anywhere = marble in civ = wonder bonus. |
|
You're right, I just checked - CityStats line 95 decides that. Then we should maybe change that to per city: "in the city where it is worked"? Would this kind of change be an argument in favour of the general architecture I suggested here? Seems not to me, I mean, woudn't make any big difference. |
Inspired by #3242: Concept on how to begin making resources more mod-able.
Questions:
! isAllowedOnTilehurt? How? (and if it stays it would need to respect those uniques too)