-
Notifications
You must be signed in to change notification settings - Fork 16
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
Implementation of a Character Faction System #36
Conversation
After some deliberation, I believe that adding dynamic player factions in this PR would make the initial factions implementation a little bit too ugly/large to merge in one go; it is a much more complicated system then what I originally expected. I'll make the earlier portions of the system ready to merge now, and the dynamic player factions implementation will be turned into an issue, to be resolved in a future PR. |
Your branch seems to be behind the current master. In this state it won't compile with the current Economy module. |
…into characterFactions
The branch has been updated with the lastest commits, it should compile correctly now 👍 |
ReviewThe PR does exactly as intended! Good job! The first thing I noticed was that the entire city spawns with just a single type of citizen (which is random). After some snooping and logging, I noticed that the SettlementRegisterEvent is received only once by the @RecieveEvent inside which resides the code for randomizing the faction alignment component. Further, I was unable to test character aggression because of the lack of different types of citizens. Another minor point is the the CitizenSpawnedEvent isn't really used. Maybe move some of the code to a method that receives that event. Also I believe there might be some confusion regarding what settlements actually are. Are they the buildings that spawn or the entire city?. After glancing through the code of DynamicCities, it seems that Settlements are the cities themselves and the way it has been used in MR is probably for individual buildings. This confusion arose because of the name given to the parameter in the @RecieveEvent - (BuildingEntitySpawned). Maybe this should be changed so that buildings are given the components and every citizen spawning inside a building is given the particular alignment too. |
Thank you for your review! First, for clarification, a settlement = a single city. A single city containing characters of only one faction type is intentional. This was done to show that a single city is "owned" by a particular faction. If this must to be changed to work better with your project this year, just let me know 😄 Faction aggression can be tested by spawning in characters using the command line. I'm going to add a testing guide for that now in the PR description. You brought up a good point about using the CitizenSpawnedEvent, I'll update that when I can 👍 I have my final exams for university this week, so I may not be able to get to this until a bit later |
I've added instructions for spawning characters to the PR description, just spawn two opposing characters next to each other and you should see the factions system in action! I investigated moving the initialization of a citizen's faction to the FactionAlignmentSystem, and I found that this would be really difficult to do without a refactor of citizen prefabs. I'll do this in a future PR, but I don't want to do that at the moment, since this will mess with other currently made PRs. |
ReviewNice Job! Finally got to see some good vs bad action. Once a bad and good Citizen were in a particular range, they both approached either and one of the gooeys disappeared. It was nice to see that the Gooeys only attacked each other when they looked at each other. The path finding again seems to be an issue here. (This one is really recurring isnt it ) Whenever the gooeys were on opposite sides of walls , or even on different heights of blocks, it just felt like the gooeys glitched when making the approach. Even after quite some testing, I was unable to figure out what the criteria was which decided which Gooey would die. (I might have to go through the code). It would be nice to have it depend on the health of the Gooeys to make it more realistic. Some animation would be nice too! |
Thank you for looking at the PR again! I'll tackle each of the points you brought up in your comment one by one:
As this pull request is currently in a functional state, I'm going to merge this now as is, and add any other improvements in a different pull request. |
Contains
A faction system which drives AI character actions. Each settlement is assigned a faction on spawn, and all characters in this settlement will be of that particular faction. When characters from enemy factions meet, these characters will be hostile towards each other.
Testing Instructions
spawnPrefab goodCitizen
orspawnPrefab badCitizen
Tasks