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

Link tech to the affected units/buildings #61

Open
ddraganov opened this issue Jun 23, 2020 · 11 comments
Open

Link tech to the affected units/buildings #61

ddraganov opened this issue Jun 23, 2020 · 11 comments

Comments

@ddraganov
Copy link

Sometimes when reading the description of a tech I wonder what exactly will be affected by it.
I am creating this issue after trying to figure out if 'Farimba' affects camel riders or not (in the description we see that it affects 'Cavalry'). Another popular confusion is the blacksmith armor lines and ranged mounted units.
I would suggest to add a list of tech ids for every unit and building in the data.json since the other direction of the relation is far less important.

@HSZemi
Copy link
Member

HSZemi commented Jun 25, 2020

While that might be possible to implement, I wonder what the UI for displaying this relation would look like. Do you have something particular in mind?

@ddraganov
Copy link
Author

Currently there is a static text listing buildings from which a unit can be upgraded. If this list is dynamic and actually listing the upgrades and includes the unique techs this will not be a big change in the UI. Also add one for the buildings.

@HSZemi
Copy link
Member

HSZemi commented Jun 27, 2020

We can currently simply take that text from the game data. By contrast, building a list of all techs that somehow affect a unit and describing the effect on that unit is

  • a lot more effort and
  • would also have negative effects on supporting multiple languages (see Add other languages #14)

@ddraganov
Copy link
Author

There will be some effort indeed. But the localization/translation will be done for the techs themself. In the dynamic list of effects you can grab the names and the descriptions from the tech list. Am I wrong?
I don't feel comfortably in this tech stack so I am a little shy to make a PR. But if you want we can chat a little online about how useful and how doable it is.

@HSZemi
Copy link
Member

HSZemi commented Jul 4, 2020

To answer the question "Does Bodkin Arrow affect Mamelukes?", you have to take the "Bodkin Arrow" Tech (id 200), look at its effect (id 193), and check each of the 93 Effect Commands if it is an Attribute Modifier (Command Types 0, 1, 4, 5, 10, 14, 15) and if it modifies either the unit itself (first value = 282) or the unit's class (second value = 12), or if it modifies a relevant resource (Command Types 1, 6, 11, 16), while knowing what all the relevant resources are. If you find a match, Bodkin Arrow affects Mamelukes. Otherwise it does not. Also, I might still have overlooked some aspects of the whole ordeal.

To answer the question "Which techs do affect Mamelukes?", you have to do that thing once for all techs that the current civilisation has access to.

Such calculations should definitely be done in advance and just the result should then be put into the data file.

The other question is: How would that information then actually be displayed in the UI?

@prateekdonni
Copy link

I was thinking something like this:
image
(ignore the actual listed research)And then ultimately, show what are the stats of the fully upgraded unit.

@ddraganov
Copy link
Author

What @prateekdonni wrote, yes. But also the building - blacksmith, castle, university, monastery. If you have this you can get rid of the static text mentioned above.

@prateekdonni
Copy link

I was thinking of research, as it may vary which one to pick, especially if in the advancement through ages, only 2/3 are available for the civilization

@HSZemi
Copy link
Member

HSZemi commented Jul 10, 2020

Implementing that is going to be a lot of work, which I personally will not tackle, at least in the near future. I am however willing to review a merge request in that direction.

@prateekdonni
Copy link

I totally understand. I will put something up.
@HSZemi 2 questions:

  1. do we get actual JSON based values for what a tech enhances or do I have to extract it from the string 😳 ? e.g. Scale Mail Armor: Infantry +1 normal armor/+1 pierce armor.
  2. I was wondering about adding classification to units? e.g. Hand Cannoneer is classified as Archer. This will help to get the above work a bit more streamlined

@HSZemi
Copy link
Member

HSZemi commented Jul 10, 2020

  1. If you want to do it properly, you get that information directly from the game data as I outlined in Link tech to the affected units/buildings #61 (comment) . You will probably need to reference the Advanced Genie Editor sources to find out which value stands for what.
  2. The unit class could be listed in the Advanced Stats section. Adding that might be a preliminary merge request for the other task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants