.

.

 

A few things right off the bat:

1. Use textmeshpro over the default text, it's crisper, more performant and just better all around.
2. I suppose this is personal preference, but I've never seen somebody simply add an empty string to something to convert an int to a string. I'd use 'money.text = moneyValue.ToString();
3. It's not exactly by the book, but because your moneyValue variable is static, when you trigger your end game screen you could grab the value and save it there. Example:

Code (CSharp):

1. // Another script somewhere managing your game
2. if(someWinCondition){
3.     EndGameScreen.SetActive(true);
4.     PlayerPrefs.SetInt("money", Money.moneyValue);
5. }

 

Reading/writing from playerprefs every frame is a very bad idea.

My suggestion would be to remove the UI logic from the update method and create two functions for getting the money value and setting it. In both methods, you could update the UI text. This will be far more performant. In addition, your last 'if' statement in the update loop will run every single frame for the entire time the end game screen is up. For a quick fix, I'd add a simple boolean to check if the game has been won. But in the future, you should look into a game state manager.

Quick fix:

Code (CSharp):

1. if (!processedEndgame && GameManager.GameIsWon) {
2.     processedEndgame = true;
3.     SaveCurrentMoney();
4. }

 

I know what you're saying. But my suggestion also shows the money going up and down. Every time you manipulate the money variables (add/remove functions), update the text then and only then. Why would you need to update the text 60ish times per second when it's not changing? In addition, you're reading playerprefs 60ish times per second, bad move.

 

It's perfectly fine to call playerprefs in awake and in any other function other than update. Also, it's also fine triggering the score update in enemy scripts. But making everything static so you can access it anywhere is a quick way to create a messy project. You've actually already got the scaffolding setup. Here's what I'd do:

Enemy script:

Code (CSharp):

1.   private void OnTriggerEnter2D(Collider2D col)
2.     {
3.         if (col.gameObject.tag == "Bullet")
4.         {
5.             col.gameObject.SendMessage("incrementScore", AddScore);
6.             Timer.timeleft += AddTime;
7.             Destroy(gameObject);
8.       }
9. }

Score script (remove static from scoreValue):

Code (CSharp):

1.     public int ScoreValue = 0;
2.     public void incrementScore(int scoreValue) {
3.         // Temporary limit to lock the range
4.         if (scoreValue < 0)
5.         {
6.             scoreValue = 0;
7.         }
8.  
9.         ScoreValue += scoreValue;
10.         score.text = ScoreValue.ToString();
11.     }

 

Another thing to note, in the code in your last post, you were passing 'scoreValue' into 'incrementScore', but 'scoreValue' is using the same case-sensitivity as your public variable 'scoreValue'. You need to start using correct case conventions.

I personally use general C# conventions like so:

Code (CSharp):

1. public int MyInt;
2. private int _myInt;
3.  
4. private void PassMeVariables(int anInt) {
5.    _myInt = anInt;
6. }

But a lot of the unity community throw C# conventions out the window and follow a completely different style. I recommend following proper C# conventions if you plan to ever use C# outside of Unity or in groups/jobs.

 

Hi NRSM

"SendMessage incrementScore has no receiver!"

I would avoid @Pendulum_ 's recommendation to use SendMessage. I'd use C# delegates and events instead, these don't need listeners to work:

Code (CSharp):

1. public static event Action<Enemy, Bullet> EnemyWasHit = delegate { };

"Also, is there a big problem keeping the static for the scoreValue?"

I don't quite see how it is related to you code block below this remark. In general, I would personally have score in some manager kind of class, like GameManager. Then enemies would have some Enemy component, that detects hits by bullets. Enemy component in enemy can then get the bullet's Bullet component, and see how much damage it caused, what it's type is. Also, enemy probably should relay the hit information and it's score value to GameManager, something like this:

Code (CSharp):

1. // In enemy component
2. private void OnTriggerEnter2D(Collider2D col)
3. {
4.     // Try get bullet
5.     var bullet = col.GetComponent<Bullet>();
6.  
7.     // Was hit by bullet
8.     if (bullet != null)
9.     {
10.         // Invoke event with this enemy and hit bullet
11.         EnemyWasHit.Invoke(this, bullet);
12.  
13.         // remove self
14.         Destroy(gameObject);
15.     }
16. }

"And there is a problem with the score/health limit, I was using "update" so the health doesn't go higher than 100 and below 0 for the score, and I have an enemy that removes the score and another that gives you health. the way it is right now the score goes negative."

You can use clamp:

Code (CSharp):

1. // Clamp score value (0 - and max value)
2. this.score = Mathf.Clamp(this.score += enemy.ScoreValue, 0, int.MaxValue);

 

@eses Although I agree with you regarding SendMessage, it is the most user-friendly way for him to accomplish his task at the beginner levels. Don't confuse him with delegates and events...