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

Force calculation fix and moddability improvement #12035

Closed

Conversation

SomeTroglodyte
Copy link
Collaborator

Fixes #12001

Side effects

Triggered Strength Uniques no longer affect force calculation
Negative Strength modifiers now do count - I saw no good reason why they were excluded
Strength uniques with "unknown" conditionals and without the new modifier no longer affect power at all

Weaknesses

  • This level of complexity might be overkill
  • Affects only BaseUnit force calc, MapUnit force eval still has little concept of Strength increases over the BaseUnit: Policies strengthening all units - nope, actual power of promotions: nope, only count of promotions is taken into account - I assume including "heal once". The modifier could be used in an overhaul, though
  • Applicability to Strength Uniques in Promotions granted through BaseUnit.promotions isn't documented, and such promotions aren't included in the new RulesetValidator warning - mostly laziness to get a good wording and trying to keep it shorter

@yairm210
Copy link
Owner

I don't like the fact that it requires micromanaging from the mod makers ('how do I know what's a good number?')
I prefer a best-effort guess that's good enough, rather than modder micromanagement
There a a few well-known conditionals that we can already assign 'relative values' to - attack/defend 50%, vs type 25%
Unknown conditionals we can assign 25% to I guess? 20%? We can always adjust later

@SomeTroglodyte
Copy link
Collaborator Author

how do I know what's a good number

Well, the modder (or we when managing the vanilla/G&K rules) will have a better idea how often a Bonus will actually apply than any hardcoded stuff - code will slowly become obsolete. And I'm sure gut feeling will give vastly better estimates than what we currently do. How is code to know that when fighting in [Foreign Land] tiles is more valuable than when fighting in [Snow] tiles? This solution can exprss that. Or take "uniques": ["[+33]% Strength <vs [Knight] units>", "[+33]% Strength <vs [Cavalry] units>"] vs "uniques": ["[+33]% Strength <vs [Mounted] units>"] - the existing power eval will weigh the former much higher than the latter though the latter is more powerful. How is code to know...

An alternate approach would be to add the AI power weight to the Conditional object itself, which would be less flexible (can't depend on parameter values) - but true, much simpler for modders. That way at least a coder adding a new Conditional that can affect Strength/power might (or might not) see there's a field to fill...

As I said I'm not entirely happy with this either, but as food for thought it's definitely something.

Oh - I forgot - this will make problems with ranking history getting "jumps" - or when ppl with different versions compare their scores... I'd not do anything about that...

@yairm210
Copy link
Owner

Hmmmmm
I'm okay with this, as long as there's some default value - say 25% or something
I do think that we should add 'sane defaults' clauses to the if, so that the cases where mod makers need to manually override is diminished

Copy link

github-actions bot commented Aug 4, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

# Conflicts:
#	core/src/com/unciv/models/ruleset/unique/Unique.kt
#	core/src/com/unciv/models/ruleset/unit/BaseUnit.kt
Copy link

github-actions bot commented Aug 5, 2024

Conflicts have been resolved.

@github-actions github-actions bot removed the Conflicts label Aug 5, 2024
@SomeTroglodyte
Copy link
Collaborator Author

as long as there's some default value

Beg to differ - we can' guess a sensible probability calculation for any new conditional, so we'll get the Wakanda mod over-accumulation sooner or later - unless the default is zero.

And dang merge tool - didn't even ask me about RulesetValidator, it runied that all by itself...

@yairm210
Copy link
Owner

yairm210 commented Sep 5, 2024

I haven't forgotten about this, I'll try to merge it into the next version
It's "better than current state" for sure

Copy link

github-actions bot commented Sep 8, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Oct 24, 2024
Copy link

github-actions bot commented Nov 3, 2024

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Force calculations getting massively screwed up in Wakanda mod
2 participants