@christiannkya
There is a dichotomy here, in that we do need to see your code, and this is a moderate amount, but at the same time when the code is beyond a certain limit in size it isn’t practicable to follow through it without being able to test it. All too many posts won’t post enough code, and while your post isn’t too much, the problem you’re facing just doesn’t lend itself easily to reading through and spotting a problem.
That said, there’s a lot to comment upon and several suggestions that will help. If you’re using Visual Studio under Windows you can use the debugger to trace through what is (or is not) happening. If you reply to inform just what debugging tools you have that will help move all of this forward. You will need to know how to enable the debugger, set breakpoints and inspect values.
I’ll turn to a few things that can help debugging. I’ll set aside the choice to use FixedUpdate instead of Update, but in either function you have to be cognizant of the fact that this function is called repeatedly (more often in FixedUpdate than Update).
These calls are independent of the mouse operation. Mouse messages come from an interrupt in the hardware, and are fed to Unity in real time. Unity accumulates mouse operations for that time when you access them in FixedUpdate, Update, Triggers or elsewhere. Mouse selection that ignores drag usually acts upon mouse up instead of mouse down, because that gives the opportunity to cancel a selection. That’s not directly associated with your bug, but it is something to consider. In any case, you have to write with a thought toward what happens when FixedUpdate is called repeatedly while a mouse button is held down, because the calls are rapid and you could be firing these related tests multiple times without really realizing it (and that could be related to your bug).
Mouse operations tend to cause “modes” of operation. Drag is an example, but so is selection. Usually there’s a plethora of actions that should not be taken when the mouse is doing something. This is what creates the mechanics which stop the accidental firing of a other behaviors of the mouse when an action has been performed. This usually means that on a mouse down, an action is initiated, and sustained as the only mouse act that can be performed until the mouse up is received. I don’t see that here.
If you’re not thinking about the repeated calls to FixedUpdate, you may think that the use of “else” for the outermost test (is it WhiteTurn or not WhiteTurn) was sufficient, but it isn’t. Consider what this code does if the mouse is held for 1/4th of a second, but fixed update is being called over 60 times per second. Let’s say mouse down 0 (left button) is down and it’s white’s turn (and tag is “White”). That fires the move in positive Z, then sets WhiteTurn to false. Turn.text becomes “Black’s turn”. So far, so good.
Now, however, fixed update fires again, WhiteTurn is false, mouse 0 is still down, and tag is still “White”. At this point the “else” fires, but since the tag is “White”, nothing happens, and that’s good, it shouldn’t. Turn.text is again set to “Black’s turn”. Obviously this isn’t your bug, so far things look correct enough, but you need to notice that setting Turn.text repeatedly isn’t good design (it wastes battery power, albeit in microscopic amounts - but continuously, doing something that doesn’t need to be done).
This is what I mean when I say the design should account for the repeated FixedUpdate cycle in the context of a real time user driven event, like the mouse button. Further, I don’t have the entire code and I can’t debug this on my machine, so I can’t say what’s happening outside the context of my reading this one bit of code. I must assume that since “tag” is a member variable, and it can be “Black”, that this FixedUpdate is running on two separate GameObjects. I can’t see this makes much sense otherwise.
If my assumption about that is correct, that changes my previous point entirely. The second version of this script would indeed see tag is “Black” and I assume would fire the move in negative x & negative z. For that to be the case I have to assume that WhiteTurn is a static value that both (presumed) scripts can see. That statement indicates why I can only go so far in analyzing what’s happening here, as I can’t really see the behavior. The analysis breaks down into too many potential directions for which I have no vision. This leaves me to discuss more generally than specifically to your root issue.
When I consider the high likelihood that this code represents two running FixedUpdate paths (one for a White and the other for a Black GameObject), my reaction is that there is only one mouse. I can’t see anything that informs me as to how that problem, of two instantiations possibly reacting against each other.
Oh, I just noticed that part of the code wasn’t formatted as code. I now realize the class is called “Mover”, and that WhiteTurn is not static. Now I can’t see how ‘tag’ can be both White and Black. The observations I’ve made still generally apply (it is still likely there are two scripts, one for each color), but now I see that the WhiteTurn is not static, and that means this concept, which is ‘global’ to the game, is held independently by (what I presume to be) two different instantiations (one for each color).
Now I turn to a bit of logic regarding the layout of the code. Your style of writing is good. In particular you make it clear what code blocks are between “if” and “else” with well placed indentation. How, however, consider what tests are happening and what code is repeated. This can cause debugging to be simpler (or more complicated when it’s a bit backwards).
For the moment I’m setting aside the nature of a repeated fixed update. I’m focusing here on repeated code and the order of tests performed.
Consider this code snippet - it is a suggestion, and example, but I’m not testing it, nor do I suggest it can be copied and pasted - use it as a suggested design to consider
// assume there are members like this
Vector3 moveTemp = new Vector3( 0, 0, 0 );
bool playerActive = false;
bool mouseDown = false;
bool leftDown = false;
void FixedUpdate ()
{
if ( playerActive == true )
{
if ( mouseDown == false ) CheckMouseDown();
else CheckMouseUp();
}
}
void CheckMouseUp()
{
if ( leftDown == true )
{
if ( Input.GetMouseButtonUp( 0 ) ) mouseDown = false;
}
else {
if ( Input.GetMouseButtonUp( 1 ) ) mouseDown = false;
}
}
void CheckMouseDown()
{
bool lbutton = Input.GetMouseButtonDown(0);
bool rbutton = Input.GetMouseButtonDown(1);
// not sure what you want, but what to do if BOTH are down?
if ( lbutton == true && rbutton == true ) { leftDown = false; return; }
if ( lbutton == true || rbutton == true )
{
mouseDown = true;
if ( WhiteTurn == true && tag == "White" )
{
if ( lbutton == true ) // due to above, one or other, not both
{ moveTemp.x = moveTemp.z = MoveDistance;
leftDown = true;
}
else { moveTemp.x = -MoveDistance;
moveTemp.z = MoveDistance;
leftDown = false;
}
rb.MovePosition( transform.position + moveTemp );
WhiteTurn = false;
Turn.text = ("Black's turn");
}
// no test needed here on WhiteTurn, it is either true or it isn't
else if ( tag == "Black" )
{
if ( lbutton == true )
{ moveTemp.x = moveTemp.z = -MoveDistance;
leftDown = true;
}
else { moveTemp.x = MoveDistance;
moveTemp.z = -MoveDistance;
leftDown = false;
}
rb.MovePosition( transform.position + moveTemp );
WhiteTurn = true;
Turn.text = ("White's turn");
}
} // if a button is down
Here are the objectives I’m illustrating:
First, the FixedUpdate is no longer where all of the code is placed. It drives the process by calling appropriate methods. This allows it to differentiate between the ‘mode’ in which the mouse is being held down, or checking to see when the mouse button transitions to the down position. That also may account for the possibility that two instantiations are running, but only 1 is active.
The mouseDown member enforces that the ‘move’ logic is only fired once. After that, where mouseDown transits to true, the repeated calls to FixedUpdate are merely waiting for the mouse to release.
This ‘CheckMouseUp’ is very ‘alpha’. The idea is to sense when the mouse button is released, but takes no care about all situations. You’ll have to think on that, if there’s any issue.
CheckMouseDown is basically what you posted, but refactored. The state of the two mouse buttons is read once and only once, such that you now have the opportunity to deal with what happens if both buttons are down (probably nothing).
Beyond that, the tests are now ‘inverted’ somewhat. If either left or right is down, mouseDown is set to true, but because the situation where both buttons are down results in an early return, it can only be left or right, not both.
Now, tests are no longer duplicated. moveTemp means that this code doesn’t create new temporary Vector3’s on each use (it isn’t necessary and just wastes memory resources). moveTemp is fashioned so that there’s only one MovePosition call for either White or Black turns. Turn.text is modified at the point that WhiteTurn is switched.
I don’t suggest this fixes the problem. I’m saying this construction, inverting the multiple tests, may make it easier to track what the bug really is. When you deal with the probability that WhiteTurn should likely be static, and that two instantiations should not be acting upon WhiteTurn (possibly with playerActive), the bug may disappear, but I don’t anticipate that.
In the meantime, inform with a reply as to your operating system and debugging tools and we can likely move forward to figuring out what’s up.