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
Personality Behaviors Template #11165
base: master
Are you sure you want to change the base?
Conversation
My couple of notes:
|
This would once again be hardcoding, maybe you can add some unique for a mod. But I don't think we should have some single if statement with no leniency for expansion as a core feature of the AI. Keep in mind that I don't know that much about the unique system.
I was thinking something like that as well. We might want to move the behaviors out of PersonalityValue to separate them from the stats. From my experience of working on with the AI all six of these are relatively easy to implement.
This PR is more of a check-in before I actually work on the changes. I thought about some others like offensive/defensive but that would require much more of a military unit AI rework. As for naming, I am good for whatever. Warmongering focus sounds a little weird, maybe conquest focus? |
Again, I'd like to point to 11160. Personalities naturally support uniques. As currently implemented, the line in NextTurnAutomation does not function with custom personalities, only neutral ones (even a blank custom personality doesn't count). And 11160 already allows for denying building settlers if it doesn't fit the conditionals. I don't see how this would add any more hard coding over what is currently available, and especially don't see how adding a specific focus for this would make anything better. Again, I kinda would rather an example of what the intent was before I could say it was a good idea. Otherwise, a move away from variables and towards uniques is generally a better idea
Still, it looks a bit off here as a PR. Personal preference, maybe Yairm or Trog but it feels like this should at least be a draft, if not an issue, and it seems a bit presumptuous as a non draft PR.
Maybe... Though the focus is more on declaring war, not so much as how you use your troops, so Idk if that's the best name for it |
I will probably change how it works. Using an integer to take multiple factors into account has worked to improve a lot of other AI functions. And having an expansion focus/behavior int would work well with that. There is no problem with a modder telling a civ it can't build more than X cities under some condition. However, I would like the base AI to be responsive (by using the integer valuing system) and expand based on its surroundings and game settings. The additions from 11160 seem like they would be a good override for modders, but not the best for AI design. also, I think I used hardcoding to refer to something more like static too much
Good point |
Well you're the AI guy. My main concern mostly stems from wanting to see implementation alongside the variables inside a PR, if you can provide that implementation, I won't have an issue Can we still convert this to a draft, though? |
Or - should use the 'discussions' feature of github? Or even projects, where you can also structure decisions, goals and assignments? Joking - I can open them to see what they look like on my fork, not here. |
I am a little confused by this, I would prefer to have this PR go through and then make incremental changes to implement the features. Commenting out the docs until it is implemented is a good idea though. |
I would prefer the opposite tbh: add in slow incremental changes implementing parts of this along the way. We can use this as reference, but we are hamstrung into saying "this must be how we do that". Avoids needing to reedit stuff like names over and over as the kinks are ironed out |
Implementing the behaviors will go in a different PR. We also need this PR to go through since it deletes the Military PersonalityValue and since the Military PersonalityValue was publicly displayed on the docs, it might cause problems since a modder could then try and use it. We can change the names as we go since we aren't showing them in the docs yet. Having to edit them is probably unavoidable, and putting everything in one PR is not a good practice. I probably won't start implementing these behaviors until after this goes through in order to prevent vcs conflicts and simplify the workflow. |
And again, as I stated prior, I'm not sure the value of doing that. From a name perspective it "militaristic" seems worse than "military" here, and the only change there is a rename, not a functional change that justifies it. Again, imo this should imo either be a draft or be an issue with a checklist, with the bulk of this discussion happening on discord or through implementation, not through here. But I could definitely be outvoted by @yairm210 |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
# Conflicts: # core/src/com/unciv/models/ruleset/nation/Personality.kt
Conflicts have been resolved. |
What's the actual diff here...? I see 'all green' and 'all red' but the before and after look the same...? |
Interesting how that happened. I added these 6 behavior attributes in PersonalityValue, Personality, and nameToVariable: |
I have no cost wonder on Military vs Militaristic tbh. |
My personal viewpoint is
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
# Conflicts: # docs/Modders/Mod-file-structure/2-Civilization-related-JSON-files.md
Conflicts have been resolved. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
# Conflicts: # core/src/com/unciv/logic/automation/city/ConstructionAutomation.kt # core/src/com/unciv/models/ruleset/nation/Personality.kt
Conflicts have been resolved. |
This PR is just a discussion of values that should go in personalities #10939 to affect the AI behaviours. There is no actual implementation yet.
I suggest these 6 attributes that can influence most of the AI's behaviours without being too specific.
This PR is just a discussion of values that should go in personalities to affect the AI behaviours. There is no actual implementation yet. My goal is to change the intention of
wantsToFocusOn()
toshouldFocusOn()
by replacing many of the uses with these new personality behaviours. Hardcoding a boolean value for the AI is almost never a good thing when it is used to directly limit what actions the AI can take.I suggest these 6 attributes to influence most of the AI's behaviours without being too specific.
The idea comes from http://civdata.com/, however, I don't agree with having so many variables that they don't mean much. These attributes need to combine enough uses so that they are actually meaningful while still allowing for a variety of play styles.
These attributes are not hardcoded to a specific victory type, but instead focus on the more general behaviour. The behaviour needs to be general in order to account for mods which may change the victory type.
Any thoughts?