Review by KerbalJeb · Pull Request #2 · Tableu/SOMAS

KerbalJeb · 2021-01-23T19:24:27Z

No description provided.

Switched from animation controller to classes/objects

…metal magic

MetalSummon: Shield will not turn with player but remain on the same side.

Removed unnecessary gameobject calls. Added comments to functions and variables. Fixed bug where metal summon would flip incorrectly

MetalSummon: negated parent rotation and removed previous variable. Completed detaching and attaching metal summon to player. Player Colliders: Improved player collision handling by adding a case for enemy collision and enemy projectiles. Switched from disabling collider during hitstun to switching layers.

- Added knockback based on location of player and enemy on hitbox collision - Added invulnerability for the player after taking damage - Added input lock for a short duration after the player is hit

- Removed old attack scripts and AttackFunctions - Refactored PlayerRaycast from functions in Update() to static functions ForwardHit and Grounded - Removed PlayerData, shifted data fields to relevant scripts and moved rotation and playerInputActions code to PlayerInput

…monobehavior from PlayerMovement: - removed playerForward and GetForward from PlayerInput. Replaced with transform.right - Changed initial player rotation to use transform.right. - As a result of changing player rotation, adjusted MetalSummon rotation and position for Left/Right

- Added EarthPunch, a close combat melee attack that shatters raised terrain in front of the player, sending fragments/shards flying at enemies. - Added Wallfragment prefab, a square projectile in the PlayerProjectile Layer.

- Moved methods and variables from EnemyData into EnemyDetection, EnemyMovement and EnemyRaycasts. - Replaced enemyData.Forward with transform.right - Rotated bones in Fire Cultist to match transform.right

- Added Stunned mode to FireCultist AnimationController - Added a case for hitting an enemy in Freeze Attack, which will stun the enemy and add an ice sprite over them. - Adjusted Fire Cultist child positions

- If the player tries to create an earth wall in the air they will instead create an earth platform that they can stand on.

- Deleted EarthMagic, FireMagic and WaterMagic. Replaced with BasicSpells and FormSpells which are attached directly to the player instead of child objects. - Replaced the element named inputactions with BasicSpell and FormSpell - Separated BasicSpells CastSpell into two parts: one for the start of a button press and one for the release. If a spell has been casted already the release will not trigger a spell.

- Added a manacost field to Projectile.cs - Set default mana costs for projectiles - Added mana field to PlayerInput, added passive regen in Update() - Added code to BasicSpells to cancel spells if the player does not have enough mana and to subtract the spells mana cost from the player's mana

- Added scaling knockback to the WaterWave spell. Knockback sends the target up and away, with knockback being stronger near the start and weaker at the end. - The WaterWave spell creates water objects where it dies. - Change EnemyMovement to apply forces instead of changing velocity directly. - When EarthPunch does not register a target it will send out a single wallFragment

…gger colliders to EnemyHealth: - Instead of a 2D sprite the flamethrower attack uses particle effects with a trigger collider. - Added OnTriggerEnter2D to EnemyHealth with the same code from OnCollisionEnter2D

… Enemy Movement, Created Demo Scene and Build: - Enemies can now detect the Player if they are hit by the PlayerInput - Created a Demo Scene and a build folder

KerbalJeb

The main things I noticed:

You can use the [SerializeField] attribute to make private variables visible in the inspector
You need to make sure that you always unsubscribe from events when an object is destroyed or you could be creating memory leaks Stack Overflow Discusison
Try to make your spell handling a bit more generic. Create one MonoBehaviour class in charge of casting spells and then create an abstract spell class or a spell interface that you can pass to your spell caster and use to implement the individual spells. Something similar to a Command Pattern would be a good place to start.
See if you can reuse the spell classes between enemies and the player so they can both cast the same spells.

KerbalJeb · 2021-01-25T17:53:31Z

Assets/Scripts/AnimationController States/ChargeWaterAnimation.cs

+using UnityEngine;
+
+public class ChargeWaterAnimation : StateMachineBehaviour
+{
+    public GameObject chargingAnimationPrefab;
+    private GameObject chargingAnimation;
+    public Vector3 displacement;
+    private GameObject player;
+    // OnStateEnter is called when a transition starts and the state machine starts to evaluate this state
+    public override void OnStateEnter(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    {
+        player = GameObject.FindWithTag("Player"); 
+        //displacement = new Vector3(displacement.x*player.GetComponent<PlayerData>().forward.x,0,0);
+        chargingAnimation = Instantiate(chargingAnimationPrefab, player.transform.position+displacement, Quaternion.identity);
+        chargingAnimation.transform.parent = player.transform;
+    }
+
+    // OnStateUpdate is called on each Update frame between OnStateEnter and OnStateExit callbacks
+    public override void OnStateUpdate(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    {
+
+    }
+
+    // OnStateExit is called when a transition ends and the state machine finishes evaluating this state
+    public override void OnStateExit(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    {
+        Destroy(chargingAnimation);
+    }
+
+    // OnStateMove is called right after Animator.OnAnimatorMove()
+    //override public void OnStateMove(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    // Implement code that processes and affects root motion
+    //}
+
+    // OnStateIK is called right after Animator.OnAnimatorIK()
+    //override public void OnStateIK(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    // Implement code that sets up animation IK (inverse kinematics)
+    //}
+}


I couldn't find where this was used in the project, but it seems ok otherwise

KerbalJeb · 2021-01-25T17:56:40Z

Assets/Scripts/AnimationController States/Idle.cs

+public class Idle : StateMachineBehaviour
+{
+    private float timer;
+    public float idleTime;


You can make this private and use the [SerializeField] attribute to make it visible in the editor

KerbalJeb · 2021-01-25T17:57:38Z

Assets/Scripts/AnimationController States/Idle.cs

+    // OnStateExit is called when a transition ends and the state machine finishes evaluating this state
+    //override public void OnStateExit(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    
+    //}
+
+    // OnStateMove is called right after Animator.OnAnimatorMove()
+    //override public void OnStateMove(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    // Implement code that processes and affects root motion
+    //}
+
+    // OnStateIK is called right after Animator.OnAnimatorIK()
+    //override public void OnStateIK(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    // Implement code that sets up animation IK (inverse kinematics)
+    //}


You can probably just delete these methods if you aren't using them

KerbalJeb · 2021-01-25T17:58:40Z

Assets/Scripts/AnimationController States/StateMovement.cs

+    // OnStateExit is called when a transition ends and the state machine finishes evaluating this state
+    //override public void OnStateExit(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    
+    //}
+
+    // OnStateMove is called right after Animator.OnAnimatorMove()
+    //override public void OnStateMove(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    // Implement code that processes and affects root motion
+    //}
+
+    // OnStateIK is called right after Animator.OnAnimatorIK()
+    //override public void OnStateIK(Animator animator, AnimatorStateInfo stateInfo, int layerIndex)
+    //{
+    //    // Implement code that sets up animation IK (inverse kinematics)
+    //}


Just delete these

KerbalJeb · 2021-01-25T18:11:27Z

Assets/Scripts/AnimationController States/StateMovement.cs

+        foreach (var t in movementMethods)
+        {
+            //Call all methods attached to the state
+            EnemyMovement.InvokeMethod(t,list);


I don't think this is really the best way to handle this. I feel like calling methods by their string name is going to cause you problems if you ever change their names. It's also much slower than calling them directly, but that likely doesn't matter too much in this case.

I would either move the functionality from Enemy Movement into this class if it isn't used elsewhere or turn it into a monobehaviour and put it on the enemy and just call the methods you need to call directly.

KerbalJeb · 2021-01-25T20:29:47Z

Assets/Scripts/Player/PlayerRaycasts.cs

@@ -0,0 +1,54 @@
+using UnityEngine;


Same as enemy raycasts, I would try and move most of this stuff to the classes where it is used (mostly player movment)

KerbalJeb · 2021-01-25T20:40:45Z

Assets/Scripts/Projectile.cs

+    private void OnCollisionEnter2D(Collision2D collision){
+        if(destructible){
+            Destroy(gameObject);
+        }
+    }


I would try to make it so that the projectile object deals damage when it hits a damageable object instead of relying on the objects to check for collisions (look into using an interface).

KerbalJeb · 2021-01-25T20:43:02Z

Assets/Scripts/Raycaster.cs

@@ -0,0 +1,21 @@
+using UnityEngine;


I don't really think this class needs to exist, just use a raycaster in the classes where it is needed instead of having a separate component for it.

KerbalJeb · 2021-01-25T20:45:43Z

Assets/Scripts/Spawner.cs

@@ -0,0 +1,23 @@
+using UnityEngine;


Not quite sure what this class does, maybe it could use a better name since it does not seem to spawn anything

KerbalJeb · 2021-01-25T20:48:12Z

Assets/Scripts/UI/HealthBar.cs

@@ -0,0 +1,20 @@
+using System.Collections;


It seems like the HealthBar and ManaBar are going to be very similar, so it might be worth trying to extract the common functionality so that it is not repeated.

…it script

Set EarthWallFragment sprite layer to Projectiles from platforms Fixed bug where using EarthPunch on an EarthWall would spawn fragments with reversed velocity.

… Created Projectiles Directory - Added WaterOrb script which inherits from Projectile and adds a rotation field - Changed PlayerHealth to GetComponent using TypeOf(Projectile)

…e child classes WaterOrbInfo and WaterWaveInfo, Chaned BasicSpells to test SpellCommands - Started to refactor BasicSpells and FormSpells into SpellCommands which uses command pattern - Should allow any gameobject to use spells from SpellCommands - Added child classes of Projectile to avoid adding extra scripts to projectiles - For example: merged WaterWave's projectile script and WaterWave script into WaterWaveInfo

…lInfo - Changed EarthCreation to use Projectile's death timer instead of Destroy - Moved all spell commands into a single class (Spellbook) instead of multiple

- Added Ice Attack, FreezeAttack and Earthpunch to SpellCommands

- PlayerPrefs stores mana and health on scene exit and loads them on scene start. - Created a scene template with a camera, canvas and global light

LevelExit -> SceneExit Level Script Folder -> Scene Script Folder

- SceneLoad spawns the player and configures the scene when there is an active scene switch. - SceneExit will Destroy the Canvas and MainCamera and transfer the Player and SceneLoad to the next scene. - SceneExit is attached to a gameobject with a trigger collider that marks the entrance to a new area. - SceneLoad and Player are marked as DontDestroyOnLoad, SceneExit changes their parameters (like position) and passes them to the new active scene.

…ontinued developing scene change system - Added Cinemachine package. Each scene has at least one virtual camera that is enabled when the scene is active and disabled when it isn't. - Added bounding boxes to each scene to control the camera - EnableVirtualCam.cs is attached to the corresponding virtual camera's bounding box to enable the virtual camera when the player enters.

Tableu added 30 commits June 20, 2020 16:33

init

3861d93

Player Animations

bc0a22b

Dark Puddle Completed, Player Knockback and Invulnerability

62a96d4

Magic System Update

b42ace9

Switched from animation controller to classes/objects

Moved old attack scripts to new directory. Set up skeleton class for …

2f2ca5b

…metal magic

Added Metal Sprites

325454e

Added comments to scripts, added feature to metalsummon

30435e1

MetalSummon: Shield will not turn with player but remain on the same side.

Adjusted code and fixed metalsummon bugs.

60f561c

Removed unnecessary gameobject calls. Added comments to functions and variables. Fixed bug where metal summon would flip incorrectly

Jetbrains changes and player hitbox adjusted

34e81a4

Applied more Jetbrains changes

1727b6f

Add files via upload

ba45935

Merge branch 'master' of https://github.com/Tableu/SOMAS

06ec085

Gitignore fix

5c27c7a

after gitignore

def8a18

Update .gitignore

6f01b79

Deleted files

18b9e91

Removing files in gitignore

22d837a

Deleted unity.gitignore

5f9ce2c

Update .gitignore

8d78c5e

deleted more files

38950b2

refresh

036def1

deleted files

5e20f66

Removing files

f0e7cc2

Update .gitignore

91e5cf0

Removing files

b82d905

Invulnerability and Hit Stun:

ab78c53

- Added knockback based on location of player and enemy on hitbox collision - Added invulnerability for the player after taking damage - Added input lock for a short duration after the player is hit

Tableu added 9 commits January 23, 2021 10:54

Added EarthPunch, Wallfragment prefab:

0b9ccf1

- Added EarthPunch, a close combat melee attack that shatters raised terrain in front of the player, sending fragments/shards flying at enemies. - Added Wallfragment prefab, a square projectile in the PlayerProjectile Layer.

Removed EnemyData:

ff319dd

- Moved methods and variables from EnemyData into EnemyDetection, EnemyMovement and EnemyRaycasts. - Replaced enemyData.Forward with transform.right - Rotated bones in Fire Cultist to match transform.right

Added IcePrison, Added Freezing Enemy to FreezeAttack:

c72de4f

- Added Stunned mode to FireCultist AnimationController - Added a case for hitting an enemy in Freeze Attack, which will stun the enemy and add an ice sprite over them. - Adjusted Fire Cultist child positions

Added EarthPlatform spell:

1d0866b

- If the player tries to create an earth wall in the air they will instead create an earth platform that they can stand on.

Added particle effects to Flamethrower Spell, Added detection for tri…

f9e3e14

…gger colliders to EnemyHealth: - Instead of a 2D sprite the flamethrower attack uses particle effects with a trigger collider. - Added OnTriggerEnter2D to EnemyHealth with the same code from OnCollisionEnter2D

Added Player Detection when damaged (Enemy), Removed StopCharacter in…

0cd02d3

… Enemy Movement, Created Demo Scene and Build: - Enemies can now detect the Player if they are hit by the PlayerInput - Created a Demo Scene and a build folder

KerbalJeb closed this Jan 23, 2021

KerbalJeb reopened this Jan 23, 2021

KerbalJeb force-pushed the master branch from 59dc8a1 to 0cd02d3 Compare January 23, 2021 20:19

KerbalJeb commented Jan 25, 2021

View reviewed changes

Tableu added 17 commits February 1, 2021 01:29

Privated fields in CameraScript, Created Beach Scene, Created LevelEx…

1156d41

…it script

Privated fields in CameraScript, Created Beach Scene, Created LevelEx…

0db1078

…it script

Changed Beach background to correct sprite layer(platforms)

0204d55

Set EarthWallFragment sprite layer to Projectiles from platforms Fixed bug where using EarthPunch on an EarthWall would spawn fragments with reversed velocity.

Added seaweed

3d13dc3

Added README

3e4c7b5

Added WaterOrb script, replaced Projectile in WaterOrb with WaterOrb,…

672fcb1

… Created Projectiles Directory - Added WaterOrb script which inherits from Projectile and adds a rotation field - Changed PlayerHealth to GetComponent using TypeOf(Projectile)

Finished refactoring BasicSpells into SpellCommands, Created EarthWal…

38cf13c

…lInfo - Changed EarthCreation to use Projectile's death timer instead of Destroy - Moved all spell commands into a single class (Spellbook) instead of multiple

Unity Version Update

b1b2e2f

Moved FormSpells to SpellCommands, Added FreezeAttackInfo

80332f0

- Added Ice Attack, FreezeAttack and Earthpunch to SpellCommands

Combined BasicSpells and FormSpells to make SpellInputHandler

b807fea

Reorganized Assets folder and added namespaces

d695768

Created PlayerPrefs.Script, Created SceneTemplate

1ee1d28

- PlayerPrefs stores mana and health on scene exit and loads them on scene start. - Created a scene template with a camera, canvas and global light

Shifted PlayerPrefs.Set/Get to individual Player files

e73f4e8

Directory changes

acbd7e1

LevelExit -> SceneExit Level Script Folder -> Scene Script Folder

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Review#2

Review#2
KerbalJeb wants to merge 100 commits intoemptyfrom
master

KerbalJeb commented Jan 23, 2021

Uh oh!

KerbalJeb left a comment

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

KerbalJeb Jan 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

KerbalJeb commented Jan 23, 2021

Uh oh!

KerbalJeb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants