Skip to content

Allow nation Faith-on-promotion uniques with XP countables - #15349

Open
Fanfblrik wants to merge 3 commits into
yairm210:masterfrom
Fanfblrik:feature/faith-on-promotion-countable
Open

Allow nation Faith-on-promotion uniques with XP countables#15349
Fanfblrik wants to merge 3 commits into
yairm210:masterfrom
Fanfblrik:feature/faith-on-promotion-countable

Conversation

@Fanfblrik

@Fanfblrik Fanfblrik commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Engine support for RekMOD Ottomans (Millets: Faith on unit promotion).
  • Wire nation/global <upon being promoted> firing from UnitPromotions (UniqueTarget stays UnitTriggerCondition for now — general unit-trigger-as-global design left open per review).
  • Countable XP required for next promotion; use existing unary ceil/floor operators.
  • Example: Gain [ceil ([XP required for next promotion] / [3])] [Faith] <upon being promoted>.

Test plan

  • UnitUniquesTests for Faith-on-promotion / XP countable
  • Manual: nation unique as above — promoting a unit grants Faith; free starting promos do not

Wire civ-level promotion triggers, accept countables in Gain [stat], and add XP-required-for-next-promotion plus ceil/floor so mods can match LekMOD Ottoman UA.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread core/src/com/unciv/models/ruleset/unique/expressions/Operator.kt Outdated
Comment thread core/src/com/unciv/models/ruleset/unique/UniqueType.kt Outdated
Comment thread core/src/com/unciv/models/ruleset/unique/UniqueType.kt Outdated

for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponPromotionGain){ it.params[0] == promotionName })
UniqueTriggerActivation.triggerUnique(unique, unit)
for (unique in unit.civ.getTriggeredUniques(UniqueType.TriggerUponPromotionGain, unit.cache.state) { it.params[0] == promotionName })

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm getting here is "any unit trigger should be available as a global trigger", I'll need to think about this, implementation is non-trivial

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this open for your design call. Runtime still fires nation/global TriggerUponPromotion from UnitPromotions so LekMOD-style Ottoman UA can work once Nation placement is allowed; UniqueTarget stays unit-only for now. Happy to dual-target only upon being promoted or wait for a general unit-trigger-as-global approach.

Drop duplicate ceil/floor functions (use unary), remove Gain docstring, keep promotion triggers unit-scoped.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Fanfblrik

Copy link
Copy Markdown
Contributor Author

Addressed what I could:

  • Removed duplicate ceil/floor functions — use existing unary operators (ceil (…)).
  • Dropped the Gain [countable] docstring.
  • Reverted upon being promoted (and gain/loss) back to UnitTriggerCondition only.

Left for your call (the non-trivial part): nation/global UA like LekMOD Ottomans still needs Gain … <upon being promoted> on a Nation. Runtime wiring via civ.getTriggeredUniques(TriggerUponPromotion) is still in UnitPromotions so it fires if such a unique is present; without TriggerCondition as a target the Ruleset Validator will warn on Nation. Happy to wait for a general "unit triggers as global" design, or dual-target only upon being promoted if you prefer that narrow exception.

@github-actions

Copy link
Copy Markdown

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

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown

Conflicts have been resolved.

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

Successfully merging this pull request may close these issues.

2 participants