So, I as trying to create an "Open world exploration" game in C# WinForms, And while coding the mining, (which works just fine), I encountered a problem with saving the number of broken blocks to the inventory (a label). Basically, for every block player breaks, it gets added to the inventory as inventoryWrite.Text = $"Grass: {grassHolder}, Rock: {rockHolder}";. Now, the thing is, sometimes, even though I use the operator, it adds up to 4 to the inventory. I'm citing the code below.

private void Remove(object sender, EventArgs e, PictureBox itm)
        {
            if (itm.BorderStyle == BorderStyle.FixedSingle)
            {
                if (itm.Tag.Equals("grass") && items.Contains(itm))
                {
                    grassHolder  ;
                    itm.Tag = "";
                }
                if (itm.Tag.Equals("rock") && items.Contains(itm))
                {
                    rockHolder  ;
                    itm.Tag = "";
                }
                if (itm.Tag.Equals("dio") && items.Contains(itm))
                {
                    dioHolder  ;
                    itm.Tag = "";
                }
                this.Controls.Remove(itm);
                items.Remove(itm);      
            }
        }

I update the inventory in a public loop, don't worry about that (interval is 1ms). But I don't think that's the problem, since I tried putting it in the Remove() function, and nothing seemed to change. I've even double locked the if statement, but nothing! It still adds more than 1. Can anybody tell me how to solve this? Thank you a lot.

EDIT:

As a reply to Ronald's comment, the if statement is called ONLY when the block is selected. ONLY once when the method is called.

CodePudding user response：

There are too many points to cover in a comment and so I've had to enter an answer.

In itself the operator is not the issue and will always behave as it should, but as someone reviewing a small piece of code the following points crop up.

1. grassHolder, rockHolder, dioHolder appear to have accessibility beyond this function and so could be altered elsewhere.
2. Function void Remove(object sender, EventArgs e, PictureBox itm) appears to be an event handler and yet there is no locking mechanism to ensure that the externally accessible parameters are not changed or used elsewhere whilst the function code is executed. Specifically items which is appears to be a collection of sorts and is used both in logic to determine whether parameters in (1) are incremented, but also has its contents changed within the function.
3. From comments made it would appear that this logic is run in response to user interaction, maybe by use of a mouse button or key event. Is this base event de-bounced to ensure that multiple triggers aren't handled?
4. Your statement "saving the number of broken blocks to the inventory (a label)." Implies that you are storing game data within the UI. This should be avoided as it ties game data directly to the UI implementation and therefore makes it difficult to alter the game, but also ties any handling of game data directly to the UI thread.

Recommended actions:

1. Ensure that the parameters in question are not accessed and altered elsewhere causing the issue seen.
2. Utilize a lock(x) statement to ensure that items is not changed whilst this function is being executed. More information here
3. De-bounce the mouse button or key click that triggers this function to ensure that multiple events aren't triggered. This is performed by placing a minimum time between event triggers. A minimum time period of say 150ms would be a good starting point. This would equate to a reasonably quick, conscious user action, but be slower than multiple events triggered by partial/poor switch contact. Incidentally this is especially true on touch screen interfaces.
4. Consider controlling access to global parameters through use of access functions. For example int IncrementRockHolder(){ rockHolder ;} Although implementation may appear onerous, they can greatly help with debugging as call stack information is then available showing what code is calling the function and thus making the change.
5. Implement a game engine class to control access to game data and implement game logic. This would allow you to unit test game functionality whilst also freeing it from UI implementation and restrictions.