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

isAI to check AI controlled players. #11979

Merged
merged 7 commits into from
Sep 29, 2023
Merged

Conversation

WCSumpton
Copy link
Contributor

@WCSumpton WCSumpton commented Sep 21, 2023

isAI added to RulesAttachment to allow for checking of AI controlled players.

Change Summary & Additional Notes

Changing isAIPlayers to isAI

isAI values: true/false, check if the player is AI controlled. Can be used with the players option.

Release Note

NEW|isAI to check for AI controlled players.

checkAIPlayers added to RulesAttachment to allow for checking of AI controlled players.
@WCSumpton WCSumpton changed the title checkAIPlayers to check AI controlled players. isAIPlayers to check AI controlled players. Sep 26, 2023
@Cernelius
Copy link
Contributor

I believe that "isAIPlayers" can be just written as "isAI" because every condition must be attached to a player (thus being a condition for that player), so having "Players" in the name is wholly redundant. Besides, not like anything else but the players can be AI anyway, so it would be redundant regardless.

Do I understand correctly that now only this option is being added (instead of three options called "checkAIPlayers", "checkAIPlayer" and "isAIPlayer"? I definitely agree with that, if so.

Have you decided if "Does Nothing" is counted as "AI" too? Would it be worth it to differentiate like in the "defaultType", where you can set "Human", "AI" and "DoesNothing"?

In particular, I think that in the "defaultType", "AI" does not comprise "DoesNothing", so that would be an inconsistency if here it does instead (I don't know.).

@WCSumpton
Copy link
Contributor Author

WCSumpton commented Sep 27, 2023

I believe that "isAIPlayers" can be just written as "isAI" because every condition must be attached to a player (thus being a condition for that player), so having "Players" in the name is wholly redundant. Besides, not like anything else but the players can be AI anyway, so it would be redundant regardless.

There is a method within TripleA already defined as isAI(). To avoid conflicting/complicating with that method was the reason for adding 'Players' to this option. As I told @TheDog-GH, the (s) on the end can be dropped, or the option could simply be called "checkAI"

Have you decided if "Does Nothing" is counted as "AI" too? Would it be worth it to differentiate like in the "defaultType", where you can set "Human", "AI" and "DoesNothing"?

This option only returns true or false. The "defaultType" or the selected value "Human/Easy (AI)/Fast (AI)/Hard (AI)/Does Nothing (AI)" are not returned. Could this option be rewritten to return the selected value? Yes, but the problem comes from what happen if the selection change, or more selections added. Also, as a map maker do you want to write separate conditions that query each setting?

I understand that changing "defaultType" to equate better to the selections, as @TheDog-GH has requested, but that is something complete different.

Thank you for your suggestions. I hope that I have answered your questions.

Cheers...

@Cernelius
Copy link
Contributor

Ok, but a question was also

Have you decided if "Does Nothing" is counted as "AI" too?

I assume that "Does Nothing" is a "false" just like "Human" for this matter, right?

@Cernelius
Copy link
Contributor

Because, whereas "Does Nothing" is (questionably) considered to be an AI in the selection screen (owning to the "AI" between brackets at the end), "Does Nothing" is considered not to be an AI in the "defaultType" (as it is implied by having an "AI" and a "Does Nothing" value for it). I think here we already have an inconsistency (unfortunately), but we should rather keep consistency with other game (XML) file things, shouldn't we?

Beside coherency issues, the other question would be whether it is actually preferable for "Does Nothing" to behave like "AI" or like "Human" for conditions.

I assume that the most normal use of this condition would be for giving additional things to a player if it is AI controlled, like what happens in "Feudal Japan" and in games in general (to make up for the inferior strategic ability of the AI compared to most humans). In this case, would we prefer that same player to receive those same things if we set it to "Does Nothing"? I think this would be a good question (and maybe also a poll) for map-makers in forum.

@Cernelius
Copy link
Contributor

I believe that "isAIPlayers" can be just written as "isAI" because every condition must be attached to a player (thus being a condition for that player), so having "Players" in the name is wholly redundant. Besides, not like anything else but the players can be AI anyway, so it would be redundant regardless.

There is a method within TripleA already defined as isAI(). To avoid conflicting/complicating with that method was the reason for adding 'Players' to this option. As I told @TheDog-GH, the (s) on the end can be dropped, or the option could simply be called "checkAI"

Other conditional options in the game (XML) file tend to be plural when they can refer to one or more things, but this is not consistent (for example, it is "directPresenceTerritories" (not "directPresenceTerritory") even though they can be one or more, but it also is "unitPresence" (not "unitsPresence") even though they can be one or more here too). However, since this option primarily refers to the player to which the condition is attached (which is only one), I second the suggestion to drop the "s" at the end (or anyhow making it singular rather than plural). Moreover, "isAIPlayers" doesn't even make sense because it should be "areAIPlayers" if you actually want it to be plural...

Regarding the presence of an other method defined as "isAI()", I don't personally see a problem as long as they apply to different matters (that is, the other method has nothing to do with conditional testing for objectives or triggers), but, if it is actually a problem, I would rather suggest a name like "isAIAssigned" or "isAIControlled" or "isAISet", to avoid the redundancy I pointed out (or "areAIAssigned" or "areAIControlled" or "areAISet" if you really want it plural).

@WCSumpton
Copy link
Contributor Author

"Does Nothing (AI)" as selected/set on the Player Setup menu would return true for AI control during gameplay. Conditions within the xml are not evaluated until after the game has started.

As to naming, I'm open to any ideas with the exception being isAI because it is already used.

Cheers...

Rename from isAIPlayers to isAI
Spacing
@WCSumpton WCSumpton changed the title isAIPlayers to check AI controlled players. isAI to check AI controlled players. Sep 27, 2023
@WCSumpton
Copy link
Contributor Author

@Cernelius

Ok isAI will work.

@TheDog-GH
Copy link
Contributor

@WCSumpton
As I dont know the implication of what Im saying below, please disregard if its not applicable.
Can isAI be used ?
As it might be the same as isAI() ?
Has isAI() "command" already been "serialized" ?

@WCSumpton
Copy link
Contributor Author

Can isAI be used ?
As it might be the same as isAI() ?
Has isAI() "command" already been "serialized" ?

Done and done. The option is called isAI while the internal method is isAi() (small i on the end).

Cheers...

@asvitkine
Copy link
Contributor

Looks great! Is there a test you can add?

@Cernelius
Copy link
Contributor

Can isAI be used ?
As it might be the same as isAI() ?
Has isAI() "command" already been "serialized" ?

Done and done. The option is called isAI while the internal method is isAi() (small i on the end).

Cheers...

It doesn't matter to me (not being a program developer), but I personally don't like differentiating in such a way. I would rather go with "isAIAssigned", which is also more descriptive of the matter.

I mean, differentiating by writing conventions is good, but (then) it should be always-applying, systematic and sensible. I'm not seeing how writing "Ai" can be a writing convention as they are both the same kind of letters (as it's like writing "Usa" instead of "USA" or "U.S.A.": it is done, but it is wrong).

I mean that differentiating something like "isAI" may be done by having "isAI" and "IsAI" (one following the convention of always starting minuscule and the other one of always starting maiuscule).

Adding RulesAttachmentTest to test isAI
Removed extra line from each file
@WCSumpton
Copy link
Contributor Author

WCSumpton commented Sep 28, 2023

I mean, differentiating by writing conventions is good, but (then) it should be always-applying, systematic and sensible. I'm not seeing how writing "Ai" can be a writing convention as they are both the same kind of letters (as it's like writing "Usa" instead of "USA" or "U.S.A.": it is done, but it is wrong).

I mean that differentiating something like "isAI" may be done by having "isAI" and "IsAI" (one following the convention of always starting minuscule and the other one of always starting maiuscule).

To the human eye yes, to the computer no. "isAI" and "isAi" are not the same. Even in the xml if a player is defined as <player name="USA" optional="false"/> then the player cannot be refenced by "Usa" or "U.S.A.". Coding is, for the most part, case sensitive.

I believe that "isAIPlayers" can be just written as "isAI" because every condition must be attached to a player (thus being a condition for that player), so having "Players" in the name is wholly redundant. Besides, not like anything else but the players can be AI anyway, so it would be redundant regardless.

It doesn't matter to me (not being a program developer), but I personally don't like differentiating in such a way. I would rather go with "isAIAssigned", which is also more descriptive of the matter.

"isAI", "isAIPlayers", "isAIAssigned" etc... This needs to be settled. A test file has already been created. Still changing at this stage can easily be done, as long as these files are not merged.

Cheers...

@WCSumpton
Copy link
Contributor Author

Looks great! Is there a test you can add?

👍

@Cernelius
Copy link
Contributor

I mean, differentiating by writing conventions is good, but (then) it should be always-applying, systematic and sensible. I'm not seeing how writing "Ai" can be a writing convention as they are both the same kind of letters (as it's like writing "Usa" instead of "USA" or "U.S.A.": it is done, but it is wrong).
I mean that differentiating something like "isAI" may be done by having "isAI" and "IsAI" (one following the convention of always starting minuscule and the other one of always starting maiuscule).

To the human eye yes, to the computer no. "isAI" and "isAi" are not the same. Even in the xml if a player is defined as <player name="USA" optional="false"/> then the player cannot be refenced by "Usa" or "U.S.A.". Coding is, for the most part, case sensitive.

By "they are both the same kind of letters" referred to "Ai", I didn't mean that "i" and "I" are the same kind of letters (as that is actually the same letter in different cases) nor that "isAI" and "isAi" are the same, but I meant that the "A" and the "I" of "AI" are of a same kind, namely they are both initials of main words as part of an acronym, meaning that they should be written both the same. The only normal way in which the "A" can be upper case and the "i" can be lower case in an acronym is that the former is a main (major) word and the latter is not. For example, "World at War" can be abbreviated as "WaW" or as "WW" but not as "Ww" and "United States of America" can be abbreviated as "USoA" or as "USA" but not as "Usa", so, if the abbreviation of "Artificial Intelligence" has a simple uppercase "A", then the "I" has to be upper case too.

What I said is that, if the choice is differentiating by writing conventions

differentiating something like "isAI" may be done by having "isAI" and "IsAI" (one following the convention of always starting minuscule and the other one of always starting maiuscule).

@asvitkine asvitkine merged commit 606deb8 into triplea-game:master Sep 29, 2023
1 check passed
@Cernelius
Copy link
Contributor

Has PoS2 been updated? It ought to.

@TheDog-GH TheDog-GH mentioned this pull request Jun 11, 2024
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.

4 participants