Combat system

Hello everyone and happy new year :slight_smile:

We have briefly started the conversation on the world interaction thread but since the combat is a different system, let’s kick off the conversation here in this new thread.
This is a system to allow attacking/damaging. When the attack input is triggered, the attack animation and sound are played (regardless of whether we are close to an enemy or not).
However, the attack will damage the enemy only when the weapon collides with the enemy. The only enemy type in the game are critters.

We also added a Card for this system on the roadmap

Happy new year @Amel-Unity
Are we using any targeting system (https://zelda.fandom.com/wiki/Z-targeting)

No, we don’t have any targeting system. The attack in the game is quite simple, the player needs to be close to an enemy but they need to be moving towards the enemy so they are facing it.

Based on your feedback I started to build a proposal for the combat system, keeping as simple as possible. First, here is a short demo using plant critter:

Basically, I have just added attack input in Protagonist script like done for jump/extra action input which is next used in state machine to trigger animation and enabling weapon.

Next, a simple Weapon script has been created to hold the strength of the weapon and also an active state, that makes the weapon armful only during attack action and not when colliding with critter during walking/running states.

Next, Critter script has been enhanced from the initial version from Sign in to GitHub · GitHub

  • _currentHealth to track health of the critter decreased during successful hit
  • GetHit boolean to track in SM a success hit between a weapon and the critter
  • Killed boolean to track in SM when _currentHeath of critter reaches zero

A logic when collision is detected to determine if the critter received an attack has been aslo added to Critter script:

The rest is just setting up SM for PigChef and PlantCritter using condition based on the new boolean or on timer and trigger actions (trigger animation, enable weapon, reset GetHit boolean when state exit)

I have opened a PR to ease the review of this combat system proposal:
https://github.com/UnityTechnologies/open-project-1/pull/292

Note: The usage of the frying pan as a weapon is just for fun :wink:

5 Likes

Thanks for your support!

Here is a new part of the system: Item dropping! First a quick demo:


To achieve this, Critter script has been updated to rely on a SO to describe the global data:
6714052--771853--upload_2021-1-11_22-33-40.png

In addition to the data, CritterSO provides some functions to compute the item dropping result:
6714052--771877--upload_2021-1-11_22-58-33.png

Just adding the instantiation of the collectable item in Critter script when the critter is dead.
6714052--771874--upload_2021-1-11_22-56-41.png

The PR: https://github.com/UnityTechnologies/open-project-1/pull/299

3 Likes

Hello,

Playing with the combat system, I was thinking about emphasizing the fact the player has hit the critter by adding a color flashing on top of the animation (again a souvenir from me playing Zelda, Ocarina of Time :wink: ), do you think it make sense here?

2 Likes

Been following the attack changes, and I definitely felt that there wasn’t enough emphasis on getting hit, but not just the enemies getting hit. I’d ask if we should do it for Hamlet as well, because when the critters fight back and nothing seems to happen to him, it doesn’t feel as impactful, especially if there is no health bar (or is there? :p).

Again thanks a lot @treivize for these great contributions!
Yes @shuttle127 , that's true for now we don't really get damaged from the attacks. This is the next step that we will work on. I think we don't really need to build it from scratch but rather reuse what we currently have to make it fit for not only the critters but also the main character.
Currently in the project we have the CritterSO that is used for the different critters and I was thinking maybe we can do some refactoring like in the screenshot below:

6745555--777640--Refactoring.png
We can separate the different proprieties in different ScriptableObjects instead of having one that has everything. This will allow us, for instance, to use health for not only the critters but also the main character.
Droppable will be also possible to reuse, I am thinking later when we add barrels that the character could destroy. The barrels will have health and droppable but not the NPC or Attack for instance.

I added the Attack so we can use it to store the attack strength for the different Critters, it is a bit similar to Weapon that we currently have for the main character.

If we change it this way, in the MonoBehaviour Critter attached to the different critters, instead of referencing one SO for everything, we can reference the different SOs needed and so on for the different characters that will need them.

What are you thoughts about this?

Hi @Amel-Unity ! That is great to see the PRs reviewed and merged!

Sure it makes lots of sense, I liked the idea to compose the behavior based on different SO following each section you have highlighted.

I would also refactor the code to move some of the logic from Critter script to the corresponding SO, not only the data, for instance the roaming destination computation and event the logic in the Update() method.
And add an interface to define a contract between Critter/NPC and SO to be able to change the roaming algo (ie roaming around starting point or roaming around a set of points, roaming along a defined path, etc.)

For attack, I think a refactor to move the reloading time from SM to the future AttackSO would be more flexible.

Regarding @Smurjo point, I was having a look at it, is it something that will be done by Unity core team or by a contribution? Currently I have the feeling that the “Getting Hit” animation for PigChef is the “Surprised” one, is it the case?

1 Like

Yes, that’s a great idea!

I think so too

Can you please specify in which thread? There is a lot of points being discussed currently and I am not sure which one you are referring to :slight_smile:

Yes it is the ‘‘Surprised’’ animation

1 Like

Sorry, actually I was referring to @shuttle127 post just above, about the player being hit by critters, linking Smurjo here was a mistake (both are so active! I wrote the name too quickly...)

About "Surprised" animation, just my opinion, but it seems a bit too painless reaction from the main character and a bit long too (maybe we can accelerate it a bit to solve this part)

Oh yes definitely, it is part of the combat system (and if we follow what we said above it can be also used with other things that are not related to the combat system, like the NPC behavior and so on…).
So if anyone from the community wants to do it, you are all welcome to do so. In this case, please let us know in this thread so we don’t all work on the same thing at the same time. Otherwise we can also do it from our side as well :slight_smile:

Yes it’s a good idea to accelerate it, at least for now and I will discuss with the team to see if we can provide an improved version for that animation later.

Hi @Amel-Unity , thanks to have demoing of the combat system yesterday even if it is not finished! That was super cool :slight_smile:

I guess it is good to add the details of the on-going PRs here in the forum.

First, as you perfectly said it is a WIP system, with incremental enhancements, each addition a mix of refactor and new things :slight_smile:
So a first part of the refactor was to remove the logical code from Critter script that was just a hardcoded state machine based on the critter status (inAlert, attacking, getHit, isDead…), so moving the wiring of the inner logic to the FSM was much more cleaner and the logic itself can be hosted in FSM conditions and actions too.

But an important part, that we discussed and agreed on with @deivsky in the PR, is keeping the capability to define variant behaviors for each critter variant without having to duplicate the whole FSM table each time. To do so, instead of defining the characteristics in the condition/action of FSM they are put in separated config SO (split as suggested above) and associated on the Critter script to be able to switch from one config to another at a prefab level.
To illustrate this capability, you can think about having a plant critter in the glade dropping apple and banana, but another plant critter in the forest dropping ginger and mushrooms. But the FSM will be the same for both. Actually, at the end the FSM for critters might remain the same for all the types of critter, only the config will change their behavior.

Again based on your suggestion, I have split the config in different SO as following:

CritterConfigSO:

  • Localized name of the Critter
  • Max Health of the Critter

Discussion:
In the other PR, my suggestion is to handle player health with a HealthAnchor, ‘a la’ TransformAnchor, to track both the max health but also the current health of the player. Player being unique, it will useful to have it in a dedicated SO.
But not sure it worth to make it deriving from another SO just for max health property, no?

So, I know that both are not related here, what could be a good refactor here? Having a MaxHealthConfigSO only for the health and move the name in Critter script directly without having another SO?

AttackConfigSO:

  • Attack strength
  • Attack reload duration

Discussion:
In the refactor PR, this attack is attached to the Critter script, but to make it working to configure also the current player weapon, I was thinking of binding it to the weapon directly, does it make sense?

ChasingConfigSO:

  • Chasing target anchor
  • Chasing speed

DroppableRewardConfigSO:

  • Drop scattering distance (how far the item are scattered on the floor from the enemy remains)
  • Dropping reward strategy

Details on the dropping strategy configuration:
Just to explain the new logic, it is now possible to define a rate for multiple item drops when an enemy is killed.
For instance, you can define: First dropped item is 100% sure and it will be 50% apple or 50% banana. Second dropped item is 20% sure and it will be 100% smoothie, etc… if an item drop fail, the algo stops to look at the other potential additional drops.

GetHitEffectConfigSO:

  • Effect Color
  • Effect Duration
  • Effect Speed (how many flashing cycle during the effect duration)

Simple flashing effect when getting hit which will play with the tinting color of the material. I know that impact effect should come soon and this one is an unplanned feature, but I feel it provides an interesting additional feedback to the player? What about doing the same on the PigChef?

RoamingAroundSpawingPositionConfigSO:

  • Roaming Speed
  • Roaming Distance
  • Stop duration

Discussion:
Based on the livestream, I understand that another roaming strategy based on predefined way points could be useful in the project. I was thinking of having the capability to define multiple roaming strategy using polymorphism on the config SO and next in FSM action, check the type of config to decide which roaming logic to play. What do you think?

1 Like

Note: I have pushed an update to the PR to add some abstractions and make the actions less dependent of Critter script.

Here is the proposal for the conifgSO split in a hierarchy of MB script:

MonoBehaviour
/
NpcEntity (Localized name, MovementConfigSO)
/
AttackableEntity (HealthConfigSO, GetHitEffectConfigSO, Expose internal state GetHit/IsDead and the Collider logic with a weapon)
/
DroppingEntity (DroppableRewardConfigSO)
/
Critter (AttackConfigSO, ChasingConfigSO)

And so “Protagonist” script is naturally inheriting from AttackableEntity, to get benefit automatically to the HealthConfig, GetHitEffect, Localized name)

Later, “NPC” could inherit from NpcEntity and “Breakable Pot” from DroppingEntity

ActionsSO and ConditionSO from FSM are now relying on these EntityClass to get the config so they can be reuse for FSM of NPC/Pot/Protagonist/Critter without having to duplicate the logic and fetch child class Critter or Protagonist.

Note 2: Abstraction has been allso added on the roaming part. There is now an empty parent class for the config MovementConfigSO and different implementation (StaticMovementConfigSO and RoamingAroundSpawningPositionConfigSO) and on the FSM action side there is one NpcMovementStrategyActionSO which will build the right IMovementActionStrategy implementation based on the type of the config (RoamingAroundSpawningPositionActionStrategy or StaticMovementStrategy)

Later it will be possible to easily add a new strategy for the patrolling over waypoints.

Hey @treivize ! I was discussing with the team your previous post just now and we saw you added other updates :slight_smile:
So I will start by answering the first post:

All this sounds good and again we have the state machine for a reason, so it’s always best to add as much as possible in there. Thanks for discussing it with @deivsky I saw the comments on your PR and I agree with that.
It’s a very good point to keep the capability to define variant behaviors for each critter variant without having to duplicate the whole FSM table each time. As a side note, with Ciro we were wondering if we will eventually incorporate the sound and visual effects in the state machine. You can notice that currently we have a Monobehaviour script called ProtagonistAudio on the pig chef. We are still discussing the advantages and disadvantages to moving sound and visuals to the state machine or keep them on the root. What are you thoughts on this?
This conversation also led to whether we want to create a state machine for different types of critters. So, for instance, we can have one for non moving critters (like the plant) and one for moving critter (like the slime), that way we don’t need to have the states related to moving for the critters that don’t move. It also means that we need to maintain both tables if we changes one of the common things between the 2 tables but again we need to see further the perks and downsides. What are you thoughts about this too?

Yes I think having the health in an independent SO is better and also we can easily use it for the UI later.

Yes let’s include the ref to the AttackConfigSO that includes the strength and reload duration to the weapon and we can call it Attack instead of Weapon (I mean for the MonoBehaviour) so we could use it for basically everything that can perform an attack in the game.

Sounds good for this part.

Sounds good for this one as well.

Yeah it’s a good idea to have a visual effect when getting hit for both critters and pig chef. We can discuss later what kind of effect but we can have like leafs blowing effect when the plant critter gets attacked or pieces of stones when we attack rock critter and so on… From the PigChef side, beside the effect when he gets hit, we can have an effect when he attacks (like when we press the attack button even in void, we can see an effect like we see in games when using a sword etc…)

Yes we can use 2 roaming strategies: roaming around a central point (in this case we need to specify the radius) or around waypoints (in this case we need to specify an array of positions, and we can create a scriptable object to store the positions like a path pattern, that way we can use the same path for multiple critters or NPC).

For this one we can create a custom inspector with an enum to actually be able to only see the properties that are related to the chosen roaming strategy. This is an enhancement we can add later in the future, we definitely don’t need it very soon.

Ok, I hope I answered everything about the first post :slight_smile:

Now in terms of MonoBehaviours, the way I thought of it with the team is to split it this way:

Damageable, A MonoBehaviour script that will be attached to any object that would receive damage. It will have a reference to

  • The HealthConfigSO (since everything that gets damaged, has a health).

  • GetHitEffectSO

  • DroppableRewardConfigSO (everything that can get damaged would potentially give a reward, except maybe the player and in this case we can put the array size to 0).

NPCAgent: A MonoBehaviour script that will be attached to slime critters and NPCs to move around. It will have a reference to

  • MovementConfigSO

For ChasingConfigSO thought to include it to NPCAgent but I think better not since NPCs will not need to chase the player or at least not all of them and all the time. So maybe we can separate the Chasing behavior in another MB script.

So as you can notice with this MBs scripts we can easily just attach them as components to the different characters depending on what we need.
So the pig chef, the critters and the barrels (or pots or anything we can break in the game) will have Damageable.
Everything that moves with the AI like the NPCs and critters will have the NPCAgent component.
Critters that move are for now the only ones who chase the player, so we can add the Chasing script to them.
Attack script that will replace the weapon script and has reference to the AttackConfigSO will be attached to the critters and the pig chef.

Let me know what are you thoughts :slight_smile:

1 Like

Honestly, I really like the state machine to define the behaviour of the characters. It is like a backbone to plug incrementally new things.
But I am not sure it is good to wire things related to the timing of animation like sound effect. I have the feeling that triggering event from the animation is easier and more accurate to configure such things. I am actually thinking of moving the “weapon” enabling from state machine to animation event to fine tune when in the attack animation the weapon is armful and not just from the start to the end of the animation. I guess we could do the same in the state machine using delay and duration, but not sure it will be as accurate in case of lagging during the attack.

Yes, so my approach is to define a unique as generic as possible state machine for all critters to avoid having to maintain several tables. Next having the configuration SO that will fine tune the behaviour for each of them. And the good thing also, it will allow to change the behaviour slightly between the same type of critter, for instance, green slime critter is roaming around a center and stop, read one without stopping always moving and blue one is patrolling following waypoints.
For the plant critter actually there are checks in FSM actions/conditions to ensure a NavMeshAgent is defined before going further with the movement so even if it’s state machines are the same for all critters it will not run the part related to roaming or chasing if it is not needed.

That is fine indeed, but just to understand, currently the weapon (attack) is attached to the cane for the player, it allows to identify the collider responsible for the damage. Do I understand properly when you said attached to the pig chef that it should be moved from the cane to the chef? In this case how differentiate the collider from the cane from any other collider defined on the chef?

Otherwise I am completely fine with all your comments, I will update the PR accordingly. Thanks a lot for this detailed review! Have a good GGJ weekend!

UPDATE: PR updated! :stuck_out_tongue:

It’s understandable. And we can keep going for now as we are still defining how attacks, animations, particles plug into the StateMachine.

But personally, I am much in favour of eventually having 3 TransitionTables, one per type of critter. This would free us from the hacks that we have in place to make the plant not move, and will probably eliminate future hacks. At the same time, it opens the possibility to program the behaviour of (say) Rock Critter and Slime Critter in a completely different way.
For instance, maybe Slime Critter never rests, it always goes to the next waypoint. The Rock Critter instead might be doing very long idles, and then going for the next waypoint. Or maybe in the future we could make it jump, by having a jump state and playing the right animation/sound/etc. so it can move up and down Navmeshes.
For now, I would move forward with just 2 TransitionTables, one for the moving ones and one for the static (Plant).

We can still have some config stuff on the base object, like the items dropped, the health, etc. so we can create variants on individual GameObjects. But I strongly feel that if we made a state machine we should use it.

Does it make sense?

No no, Amel was just referring to the fact we discussed we could rename the Weapon script to a more generic Attack, since we think it’s a script that goes on both the pig’s cane but also on (maybe) invisible colliders which represent the damaging colliders of critters. (this could also apply to something like a trap, or an environmental hazard)

But no, we won’t move the script to the character’s root or as you say, we wouldn’t be able to call an OnTriggerEnter on the right collider from there.

1 Like

Hi @cirocontinisio , it makes totally sense. Indeed having one state machine table per type of critter might be more reasonable. Actually currently I am just having the feeling they will be not different enough to do the split and maintain the states in double. But definitely it would offer more flexibility at the end. And not sure it will be that heavy to maintain them.

And sorry for my confusion on the last part, we were actually perfectly aligned from the beginning :slight_smile:

So regarding what has to be made based on your comment:

  • Split the transition table to have one for the plant, one for the slime
  • Keep the usage of variant configuration for health, attack, drop rate, get hit effect (I guess it will make sense to customize it for each slime critter variant)
  • Integrate the roaming strategy and configuration directly in the FSM action without injecting the strategy via the config
  • Integrate the chasing config in the FSM action

Maybe not the walking ones, at least at the moment. In fact let’s keep them together for now.

Yes. Are you going to do it as part of the PR already up, or should I start accepting that…?

(thanks, as usual)

Thanks for the quick feedback a Saturday evening! I am integrating these changes in the first PR right now, so, it should be fine to have a final review after my next commit :slight_smile:

1 Like