ConditionalDisjunction

[dHannasch]

This doesn't actually work, this is just to follow up on discussion. As you can see, implementing "This policy cannot be adopted unless either Theology is discovered or a religion is founded" would be quite straightforward on the back end, unless I'm missing something. The hard part is the text parsing. I don't know how to make that work.

The reason it doesn't work is because https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/models/translations/Translations.kt#L492

fun String.getConditionals(): List<Unique> {
    if (!this.contains('<')) return emptyList()
    return pointyBraceRegex.findAll(this).map { Unique(it.groups[1]!!.value) }.toList()
}

doesn't allow nesting conditionals. (Obviously, once everything is parsed into objects, the Unique objects do allow nesting conditionals.)

[SeventhM]

Just as a note, my statement would be that to cover everything you want, Ideally the following should be possible

• [ ] Only available <after discovering [tech] <conditional>> should not have the ConditionalTech condition be valid if the second conational is false when checking something along the lines of unique.condtionals
• [ ] Only available <after discovering [tech> <conditional> should have the ConditionalTech condition be valid if the second conational is false when checking something like unique.conditionals, but should not be considered valid in situations like a requiredTechs function. This keeps consistency with current modder expectations. I'm aware that something like .filter{ it.type != ConditionalTech && Unique.conditionalApplies(it) works, but this is clunky to need every time and should be available either as a part of getMatchingUniques or as a helper function
• [ ] Some opinion needs to be had on Only available <conditional <after discovering [tech]>>. The non programmer expectation likely is that it matches in all situations as the first scenario

[SeventhM]

quite straightforward

Also, wouldn't say this much. Currently, all checks through the uniques do not care about unique or conditional order (any effects based on order is accidental). As implied, you've now added order as relevant for conditionals

[dHannasch]

Only available <after discovering [tech] <conditional>> should not have the ConditionalTech condition be valid if the second conational is false when checking something along the lines of unique.condtionals

I have no idea how to parse what you're saying here. What? Are you using "valid" to mean "true"?

Currently, all checks through the uniques do not care about unique or conditional order

Which I think is a very good idea.

As implied, you've now added order as relevant for conditionals

Um...no? There's only one added check here.

UniqueType.ConditionalDisjunction -> condition.conditionals.any{ conditionalApplies(it, state) }

You can see that order remains 100% irrelevant.

Only available <either <after founding a religion> or <after discovering [Theology]>>

is exactly the same as

Only available <either <after discovering [Theology]> or <after founding a religion>>

By order mattering, do you mean conditionals nesting within other conditionals? They already do. If I'm reading correctly, every conditional has its own conditionals list of sub-conditionals. Of course, that currently isn't used because the text-parsing won't parse nested <<><>>. (Which surprised me, since again, every single conditional currently carries around a list of sub-conditionals. It's just that...I think...that list is always empty. I assume there's some complicated history there. In any case, the infrastructure is already there if we parse into it.)

Any bit of code calling conditionalApplies would never know or care that conditionalApplies was recursing through some more complicated structure under the hood.

[SeventhM]

Um...no? There's only one added check here.

Am referring to the <either>, though I may have misunderstood given the simplistic wording. Tbh, I didn't look at the implementation

Only available <after discovering [tech] >

I'm saying unique.conditionals (or some equivalent once you've done) should not include the provided ConditionalTech if the nested <conditional> comes up false. There should be a way for that to easily check for this in such a scenario. That's kinda the natural assumption from a modder point of view of what a nested conditional means. The assumption likely isn't that the ConditionalTech is false in the case (though I could be wrong). At the very least, the assumption should be that it's not true or false if the idea is to have feature parity with Requires the [tech] tech <conditional>

Edit: huh...<> doesn't work in messages without escaping it or using backticks. Didn't know that

[SomeTroglodyte]

#10076 might interest you...

huh...<> doesn't work in messages without escaping it or using backticks

Because github doesn't do classic markdown, it allows a mix of md and a few html tags (regrettably no style attributes, so colored text only via images). We're using <details> quite a lot. And automatic interpretation of @ and # isn't standard markdown either.

[yairm210]

This is kind of brilliant in a 'yo dawg I heard you like' way, but there's no way for the end user to know if his "A if B if C" means AND or OR

[dHannasch]

"A if B if C"

I'm not sure what you're talking about?

Only available <either <after founding a religion> or <after discovering [Theology]>>

Where are you getting "A if B if C"?

[SomeTroglodyte]

What I don't see is where the or used in the examples comes into play in the implementation... I suppose that's left open 'cuz the nesting issue is unanswered.

Hint: we now have UniqueType.docDescription and this one would merit some. If it goes forward that is.

So - replace the parser ( _{^{possibly with the #10076 one}} ) or not and forget this...

[github-actions[bot]]

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

[github-actions[bot]]

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[bot]]

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