Add Custom Tiles for Ancient Era Technologies in the Science Adviser Screen

[FranklinFFR]

Half of the ancient techs (10 of them) now have 32x32 PNGs produced by me (no AI involvement).

Currently, the Krita files are also included for all but one (Literature's is a downscaled photograph I took) of the images, which documents both their human creation, and makes it easy to adjust in the future.

[stavrosfa]

I would do it more like this

if tostring(tech.id) == "tech-3" then return {path = "Art/Tech Chooser/Icons/Alphabet.png",} end

using the tech id instead of the name, it would be easier to maintain and not have to worry so much about typos or updating the tech names here as well.

In a mod, "Alphabet" could become "Scavanging" for example, but still having the id "tech-3", so in Lua the only thing that the modder would have to change would be the icon name, not the tech name as well.

Since these are also close to 100 icons, I would keep each entry as one line (still using if/else, not just if) to avoid all the noise from the lua syntax. I think this will compile just fine too.

@WildWeazel how can we get these assets in the Assets repo? Does Frankiln need to send the files to us, or could we grant some kind of access to open a PR there?

[esbudylin]

I would do it more like this

if tostring(tech.id) == "tech-3" then return {path = "Art/Tech Chooser/Icons/Alphabet.png",} end

using the tech id instead of the name, it would be easier to maintain and not have to worry so much about typos or updating the tech names here as well.

If the goal is to reduce the syntax noise, wouldn't it be better to just use a table for this?

Like

local tech_icon_replacement_map = {
  ["tech-3"] = "Art/Tech Chooser/Icons/Code of Laws.png",
}

....

function c7_textures.tech_icons.small:map_object_to_sprite(tech)
  local icon = tech_icon_replacement_map[tech.id] or "Art/Tech Chooser/Icons/placeholder.png"
  return { path = icon }
end

(of course, the path prefix can be shortened further).

Regarding the Assets repo, I think there is no restrictions on opening PRs to it, but since Franklin is interested in making art contributions, we should probably just give him commit access to it (the changes to the Assets repo should go through the PR to the main repo anyway). @WildWeazel, what do you think?

[stavrosfa]

Yeah, i didn't think of that, good point.

Also, I was just thinking too, that the "Art/Tech Chooser/Icons/" part could become a constant.

[FranklinFFR]

I like @esbudylin 's approach for scaling the elseif chain.

@stavrosfa is there a spot I could see the definitions of the techs (list of ID matched with name) so I don't need to guess and check the ID's for all of the techs?

[esbudylin]

is there a spot I could see the definitions of the techs (list of ID matched with name) so I don't need to guess and check the ID's for all of the techs?

You should check this section of the ruleset file:

OpenCiv3/C7/Lua/civ3/ruleset.json

Line 11271 in d1d5986

"techs": [

[FranklinFFR]

Took me a while to figure out why tech.id directly wasn't working, but solved the issue by wrapping it in a tostring(). Also abbreviated the directories of the images.

[WildWeazel]

I definitely agree with Yegor's (good to see you back) approach of mapping the ID and using a function to fall back to a safe default. Not sure about making the directory a constant - do we want to force people to put things in specific locations and spare them duplicating full paths, or to give them the flexibility to keep packages of custom assets together and make them be explicit? I think I'd favor the latter as a rule, but that's a separate topic.

But anyway (and I still have not read the previous big PR for Lua changes so bear with me here),

is there a spot I could see the definitions of the techs (list of ID matched with name) so I don't need to guess and check the ID's for all of the techs?

this is already highlighting my issue with how this is coming together. Why should we have to refer elsewhere to figure out what the IDs are? Why isn't all the data corresponding to one thing in the same place? You know how in Civ3 it's annoying that some data is in the BIQ, including even some media in certain cases, while the rest is spread across several places in pediaicons and civilopedia? It feels like we're going down the same path here. I don't want to make non-technical modders have to hunt through Lua scripts and update a bunch of things in multiple places and potentially multiple scripts, and have to avoid breaking the syntax.

IMO we encapsulate all of the raw data for each thing in a JSON object and reference fields from that in Lua. I know we can't co-locate rules with media given the way standalone mode is currently implemented (another topic...) but to borrow from the model-view-whatever paradigm there could be a reference in each thing's rules json section to an object ("thingId_assets" or whatever) to be found in another json file (which would be swapped out depending on our mode) that contains no logic but just the file paths, etc for all the content associated with that thing in one place. Then the Lua for assets can be totally data agnostic aside from fallbacks, and just plug in the values from each thing's asset json. And nobody is forced to edit Lua whenever they change a graphic.

Side note (yet another topic), yes we should use IDs to reference things, but even now we're setting precedents that people will most likely follow by imitation. Generic IDs like "tech-n" are not descriptive, and will not be unique once people starting trying to merge content from different mods as I would want to support as much as possible. "tech-alphabet" gets the point across, but I'd go as far as "scoping" our built-in data like "oc3.tech.alphabet" to identify the one from the base game rules, vs a very different one that might be found in someone's mod. The actual display name might drift from the ID over time, especially with translations (topic!), but at least it is clear and unambiguous.

Alright, tell me why I'm too out of the loop to know what I'm talking about :)

[FranklinFFR]

After figuring out how the Tech object works to make the ID matching work, I didn't think of just putting the directory in the json, but that sounds like the obvious best move. I can try my hand at doing that, though I may end up with a not so elegant solution for the placeholder depending on how things go.

edit: after discussing it, it seems that we should go ahead and merge, and maybe come back later if we decide on how we are going to manage custom assets

[esbudylin]

Can you remove this stray comment? Otherwise, looks ready to merge.