koobas.hobune.stream
if( ... && ... && ... || ... &&) { reload }
So I have this code for my reload function
if(Input.GetKeyDown("r") && magAmmo < magCap && ammo != 0 && reloading == false && zip.GetComponent(HangOnObject).hanging == false && player.GetComponent(JumpAnimate).jumping == false){
reloading = true;
but if I go into another scene reload won't work because zip no longer exists. I tried
if(Input.GetKeyDown("r") && magAmmo < magCap && ammo != 0 && reloading == false && zip.GetComponent(HangOnObject).hanging == false || GameObject.Find("Connect") == null && player.GetComponent(JumpAnimate).jumping == false){
reloading = true;
but when I go into another scene he won't stop reloading. What can I do?
edit: I tryed this also
if(Input.GetKeyDown("r") && magAmmo < magCap && ammo != 0 && reloading == false && zip.GetComponent(HangOnObject).hanging == false && player.GetComponent(JumpAnimate).jumping == false
|| Input.GetKeyDown("r") && magAmmo < magCap && ammo != 0 && reloading == false && GameObject.Find("Connect") == null && player.GetComponent(JumpAnimate).jumping == false){
reloading = true;
still won't reload on other scenes.
It's generally never a good idea to have && and || operators in a single row. I would wrap your conditions in brackets so it's much more obvious which conditions do what.
There are lots of optimization opportunities here, but it might be easiest to do something like this until you have it all under control.
if ( Input.GetKeyDown("r") && canReload() ) {
reloading = true
print("we are reloding...");
}
private bool canReload(){
// can't reload if we are already reloading
if ( reloading ) { return false; }
// can't reload if we are out of ammo
if ( ammo == 0 ) { return false; }
// can't reload if we fail some "mag test"
if ( magAmmo >= magCap ) { return false; }
// can't reload if we are jumping
if ( player.GetComponent(JumpAnimate).jumping ) { return false; }
// can't reload if we are on a zip line
if (
zip != null &&
zip.GetComponent(HangOnObject) != null &&
zip.GetComponent(HangOnObject).hanging ) { return false; }
// Looks like we can reload
return true;
}
You don't need != null. A reference type is automatically considered as false if it references null.
It would also be more efficient if you got the HangOnObject once before the condition, rather than twice during.
The JumpAnimate component can also be cached.
The conditional check is complicated so my advice is to make it as straight forward as possible before attempting something clever.
In my opinion testing for a null is much clearer than testing to see if something can be cast to bool.
Of course it would be more efficient to NOT GetComponent() multiple times a frame. That was not the point.
First off, its a very bad idea to use GetComponent every frame; store it on start and access it every frame.
Second, here's your code with line breaks so I can refer to it easier:
if(Input.GetKeyDown("r")
&& magAmmo < magCap
&& ammo != 0
&& reloading == false
&& zip.GetComponent(HangOnObject).hanging == false
&& player.GetComponent(JumpAnimate).jumping == false)
{
reloading = true;
}
if(Input.GetKeyDown("r")
&& magAmmo < magCap
&& ammo != 0
&& reloading == false
&& zip.GetComponent(HangOnObject).hanging == false
|| GameObject.Find("Connect") == null
&& player.GetComponent(JumpAnimate).jumping == false)
{
reloading = true;
}
Now, you say that zip no longer exists. If you know that for sure, why use line 16 at all? I think the OR is definitely the problem because I have no idea how it would handle that. Typically you'd combine clauses in parentheses with an OR to handle specific cases:
(A && B && C) OR (!A && D)
My overall suggestion though, get all the components at start and then figure out your logic. I'd suggest something like this :
HangOnObject hanger ;
JumpAnimate jumper ;
GameObject connector;
public bool hasZip;
bool hasConnector;
void Start(){
if(hasZip){
hanger = zip.GetComponent<HangOnObject>();
}
jumper = player.GetComponenet<JumpAnimate>();
connector = GameObject.Find("Connect");
if(connector == null) hasConnector = false;
}
void Update(){
if(Input.GetKeyDown("r"){
if( magAmmo < magCap // player isn't already loaded
&& ammo !=0 // there's ammo left to use
&& !reloading //I'm not already reloading
&& ((hasZip && !hanger.hanging) //zip exists and im not hanging
|| (!hasZip && hasConnector))
&& !jumper.jumping //I'm not mid-jump
){
reloading = true;
}
}
}
I didn't figure it out but this helped me organize a lot, so thanks.
updated with a condition about having the zip component...but I like @jonSG's answer better ( except you should still get the components at the beginning)
You should try introducing brackets to your multiple conditional statements to show 'priority'.
2*3+2-5
(2*3)+(2-5)
2*(3+2)-5
The three arithmetic statements above will give different answers.
if(isWriting==true && isStanding==false || uNum==2){
}
is different from
if(isWriting==true && (isStanding==false || uNum==2)){
}
Try placing your ANDs and ORs in their own groups of if statements. I've had trouble by placing && || && and it doesn't work right. So try placing all your && in one, and your || in another.
