yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.26k stars 1.55k forks source link

Better tech modification support #2221

Closed Naprogramuji closed 4 years ago

Naprogramuji commented 4 years ago

I was able to add new tech using mod support but have some ideas for improvement of it:

  1. Tree should be generated dynamically. Row and column must be hardcoded right now and there should be problem with mods conflict in future. There is line crossing new technology on screenshot due to this hardcoded tree version as well.
  2. Era, tech cost, building cost and wonder cost must be part of new tech data. But there should be column and row as location identificator only. These values should be loaded from original when not overwritten. Or just required technologies and era in case of dynamic tree. Again. It can cause problem with mods conflict right now.
Snímek obrazovky 2020-03-21 v 21 53 56
Naprogramuji commented 4 years ago
  1. It is not possible to use "requiredTech" for building from core files withou placing it into Techs.json file in mod. Again.
rh-github-2015 commented 4 years ago
1. It is not possible to use "requiredTech" for building from core files withou placing it into Techs.json file in mod. Again.

Yes you can - provide the cost property explicitly and it won't need to access the tech at ruleset load time

rh-github-2015 commented 4 years ago

Tree should be generated dynamically

I estimate 3 to 5 man-days for someone with really good skills... Maybe if you googled algorithms one could plagiarize - didn't that non-sid-meyer civ clone, CTP, get open-sourced or leaked?

rh-github-2015 commented 4 years ago

CTP2 sources if anyone wants to have a go. I'm 33% sure it had a tech tree like that... Still got the CD of the original... Lawyers!

Sorry I didn't merge my comments, you probably got 3 mails instead of one.

Naprogramuji commented 4 years ago

It's not technical problem mainly. I did this kind of dynymic tree stuff for my clients. I can help with it too in my free time. I was add brewery as prerequisite tech for philosophy and looks fine right now. I think this task is not priority right now.

JackRainy commented 4 years ago

brewery as prerequisite tech for philosophy

Haha! No beer - no philosophical thoughts! True story, bro! 🍺

Naprogramuji commented 4 years ago

;) "Na zdraví" 🍺

Naprogramuji commented 4 years ago

Ribbon should expand automatically as well depending on count of icons, padding above tech name.

Snímek obrazovky 2020-03-27 v 4 52 15
Naprogramuji commented 4 years ago

And search input should be nice.

Naprogramuji commented 4 years ago

I'll add another stuff for my mod and will start on this task after finishing it. Or someone can start it first and I can pull requests to his branch.

Naprogramuji commented 4 years ago

Ok. Trying doing some stuff on it.

Naprogramuji commented 4 years ago

https://github.com/Iver88/Unciv

Naprogramuji commented 4 years ago

Better padding. Longer tech names (mainly in translations or names with diacritic) are not glued to the right or top edge. 4th icon has bigger padding too right now.

Snímek obrazovky 2020-03-28 v 21 55 11 Snímek obrazovky 2020-03-28 v 21 55 17 Snímek obrazovky 2020-03-28 v 21 55 22
yairm210 commented 4 years ago

There are a bunch of things here About button length, JackRainy dealt with that I managed to mod a tech quite simply, I see that there could be a demand for forgoing bringing in the 'column' wrapper but I think it works well as-is - you can just have the column wrapper and in it only the tech you want to add/edit To be totally honest, I think that building a dynamic tree will lead to a worse tech tree, from an aesthetic point of view. There is a lot of deliberation in placing the current techs exactly where they are. But if you want to have a shot at it, go ahead! About "you can't reference original techs from mods" - this is a GOOD POINT, and I'll try to deal with that