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

Countable comparison conditional uniques #11308

Merged
merged 3 commits into from
Apr 1, 2024

Conversation

PLynx01
Copy link
Contributor

@PLynx01 PLynx01 commented Mar 14, 2024

As I promised two days ago #11280 , I've made a pull request which contains five new uniques for comparing countables.
Countables can be numbers (both integer and float) and numeric variables. For now, only these variables are supported:

  • Current year
  • Stat amounts
  • Resource amounts

If you have more ideas for the countables, give me a lot of them, as adding new countables is a quick and easy process.

I would also like to add two TriggerCondition uniques, which are true when the number of one countable becomes higher or lower than another countable. Please give me some advice about it.

Better yet, we can add support of nested parameters or even evaluation of mathematical expressions into numbers, either by using this https://github.com/notKamui/Keval or similar library or our custom code for that task.

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 15, 2024

OK, I've applied your suggestions. But there are more countables than just stats, resources and years. Should I left it as it is now, or should I add support for more countables? If so, please give me some ideas, so I can implement them.

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 18, 2024

@yairm210 I've applied your suggestions. What else is necessary to merge my PR?

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 25, 2024

@yairm210 Please review!

ConditionalCountableDifferentThan("when number of [countable] is different than [countable]", UniqueTarget.Conditional),
ConditionalCountableGreaterThan("when number of [countable] is greater than [countable]", UniqueTarget.Conditional),
ConditionalCountableLessThan("when number of [countable] is less than [countable]", UniqueTarget.Conditional),
ConditionalCountableBetween("when number of [countable] between [countable] and [countable]", UniqueTarget.Conditional),
Copy link
Owner

Choose a reason for hiding this comment

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

"is between"

Copy link
Owner

@yairm210 yairm210 left a comment

Choose a reason for hiding this comment

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

Other than one minor text issue, looks mergeable!!

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 26, 2024

@yairm210 Check out my PRs. I've made some corrections

@woo1127
Copy link
Contributor

woo1127 commented Mar 27, 2024

@PLynx01
When number of [Iron] is between [Horse] and [Niter] still sound weird to me, but if Yairm finds it acceptable, I have no issue with it either.

My actual inquiry is whether we can enable these uniques to accommodate (modified by game speed) or (per turn) ?

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 27, 2024

@PLynx01
When number of [Iron] is between [Horse] and [Niter] still sound weird to me, but if Yairm finds it acceptable, I have no issue with it either.

My actual inquiry is whether we can enable these uniques to accommodate (modified by game speed) or (per turn) ?

What do you mean by saying (per turn)? I suppose you're talking about stat income or growth of resources per turn.

I am right?

@woo1127
Copy link
Contributor

woo1127 commented Mar 27, 2024

What do you mean by saying (per turn)? I suppose you're talking about stat income or growth of resources per turn.

I am right?

Yes that what I mean by per turn, the stat income or growth of resource per turn.
Because now all the conditional uniques only refer to the current accumulated amount of stat
For example when above [100] [Gold] is not the same as when above [100] [Gold] per turn

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 27, 2024

What do you mean by saying (per turn)? I suppose you're talking about stat income or growth of resources per turn.
I am right?

Yes that what I mean by per turn, the stat income or growth of resource per turn. Because now all the conditional uniques only refer to the current accumulated amount of stat For example when above [100] [Gold] is not the same as when above [100] [Gold] per turn

In that case it should be parameter not a unique

@woo1127
Copy link
Contributor

woo1127 commented Mar 27, 2024

In that case it should be parameter not a unique

If so we can introduce the concept of statResourceModifierFilter !!!

For instance, unique like Gain [3] [Culture] (modified by game speed) can be change into Gain [3] [Culture] [statResourceModifierFilter], where statResourceModifierFilter could accepts the options I previously mentioned:

  • modified by game speed
  • per turn
  • any other modifiers you can imagine

Then your new uniques would also become when [countable] is between [countable] and [countable] [statResourceModifierFilter]

What do you think ?

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 27, 2024

@woo1127
There is also another dimension: Scopes of countables

They are

  • Global (all civilizations)
  • Civ-wide
  • City-wide
  • Unit-wide

And also diplomacy-related scopes

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 27, 2024

The template would be something like this:

when [scope] [countable] is larger than [scope] [countable]

@woo1127
Any thoughts?

@woo1127
Copy link
Contributor

woo1127 commented Mar 27, 2024

Can I have few examples to clarify what you mean ?
We can just use cityFilter for civ and city wide while unitFilter for unit wide isn't it ?

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 28, 2024

Can I have few examples to clarify what you mean ?
We can just use cityFilter for civ and city wide while unitFilter for unit wide isn't it ?

That's a good idea. We can use filters to define scope. But we need to group countables by these scopes:

  • Game-wide countables
  • Civ-wide countables
  • City-wide countables
  • Unit-wide countables

@PLynx01
Copy link
Contributor Author

PLynx01 commented Mar 28, 2024

Can I have few examples to clarify what you mean ?
We can just use cityFilter for civ and city wide while unitFilter for unit wide isn't it ?

That's a good idea. We can use filters to define scope. But we need to group countables by these scopes:

  • Game-wide countables
  • Civ-wide countables
  • City-wide countables
  • Unit-wide countables

@woo1127
Here is the list: (long but non-compete)

Game-wide:

  • Current year
  • Current turn
  • Years since beginning
  • Number of all civilizations ever present
  • Number of current civilizations
  • Number of cities of all civilizations
  • Number of tiles (with tile filter)
  • Global population

Civ scope (supports Game-wide scope)

  • Number of cities (with civ filter)
  • Number of cities (with civ and city filter)
  • Total stats per turn (stat filter, optionally civ and city filter)
  • Total stockpiled stats (gold, science, culture, faith)
  • Total stats ever produced
  • Total resources currently stockpiled
  • Total resources ever stockpiled
  • Number of traded resources
  • Number of wars
  • Number of defensive pacts
  • Number of science agreements
  • Number of wonders built
  • Number of units
  • Civ population

City-wide (supports civ and game wide)

  • Number of buildings
  • Number of wonders
  • Stat production per turn
  • Worked tiles
  • City population
  • Turns until population growth/construction complete/border expansion

Unit-wide (supports previous scopes)

  • XP
  • Base strength
  • Max strength
  • HP
  • Number of promotions

@yairm210 yairm210 merged commit 5e89e2f into yairm210:master Apr 1, 2024
4 checks passed
@woo1127
Copy link
Contributor

woo1127 commented Apr 1, 2024

@woo1127 Here is the list: (long but non-compete)

100 hours of work would be required to implement all of these 😅💀

@PLynx01 PLynx01 mentioned this pull request Apr 4, 2024
2 tasks
@PLynx01 PLynx01 mentioned this pull request Apr 27, 2024
2 tasks
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.

3 participants