- Home /
Using a lot of calls and if else statement, is there a better way to do this?
Hi there, I'm still trying to figure out what is the best practice when it comes to game scripting in unity.
The script I've done displays different text on UI when a gameobject is detected from raycast. As you can see below, it is pretty lengthy.
Is there a better way to do this? Any pointers or advice is much appreciated!
if(myScript.selectedObject.isEnemy || (myScript.selectedObject.isEnemy && myScript.selectedObject.isDestroyable))
{
displayText[1] = inGameMonologueScript.canKill;
}
else
{
displayText[1] = inGameMonologueScript.notEnemy;
}
if(isScenarioScript && isScenarioScript.scenarioBegun)
{
displayText = isScenarioScript.scenarios[isScenarioScript.currentScenario].scenarioDialogue;
}
Answer by Kossuranta · Feb 23, 2017 at 11:23 AM
That should be just fine. You said that there are a lot of calls so is this inside Update method? Because you maybe could call it after selectedObject has changed, no need to do something like that on every frame.
Other thing I noticed that your first if doesn't really make sense. Later part of it will never affect anything as it will be only checked when isEnemy is false and if it's false then the later part will also always be false.
Did you mean for it to be something like this, where if isEnemy is false then it checks if isDestroyable is true and only if both are false then else is called.
if(myScript.selectedObject.isEnemy || myScript.selectedObject.isDestroyable)
If you wanted to check that if isEnemy is true and isDestroyable is also true then it should be like this:
if(myScript.selectedObject.isEnemy && myScript.selectedObject.isDestroyable)
Hey! Thanks for the tip! Yes, that happens in update, since the selectedObject is obtained via raycast, however, when an input is made for instance On$$anonymous$$ouseDown, several functions gets called at once and it has a lot of calls like the example above to change the variables of the gameObject.
For the first part, yes you pointed out right! :D I didn't realize the silly mistake there. But I do have other scripts that looks like this....
if(myScript.selectedObject.isEnemy || myScript.selectedObject.isDestroyable || myScript.selectedObject.is$$anonymous$$ale || myScript.selectedObject.isFemale || myScript.selectedObject.isChildren || myScript.selectedObject.isOldman)
selectedObject can be a lot of things, I do this when I want to change other variables for certain objects that has this script attached to it. It looks okay if it only affects one type, but if 6 out of 12 has to be affected. I could only think of this method.
This example looks like you aren't abstracting enough in what you are trying to do.
1) If you are checking all of those to see if you can move the object, then you can ins$$anonymous$$d make a variable "is$$anonymous$$oveable" and just check to see "if (is$$anonymous$$oveable)".
2) You have several different 'categories' there, and if anyone one of them is true your script fires. This seems really improbably and prone to bugs.
isEnemy is an alignment status check,
isDestroyable is a potential state check,
is$$anonymous$$ale and isFemale cover both genders (so only non-genders would fail to fire),
isChildren is checking either genderless relative age, plurality, and/or ancestry relationship (or all of the above) isChild makes more sense for a single object variable
isOldman is a relative age AND gender at the same time
I'm hoping this was just an example, but if it's not, you really need to try to group them into categories, and only check against things in the same category as each other.
3) I don't really like C#'s options for handling long if criterias, but if you needed to check against a long list of potential GameObject tags (sounds like you should probably look into their usage https://docs.unity3d.com/$$anonymous$$anual/Tags.html) I'd probably do something like:
string[] possibleTags = new string[6] {"Enemy", "Destroyable", "$$anonymous$$ale","Female", "Child", "Oldman"}
if (possibleTags.Contains(myScript.selectedObject.tag) {
//do something;
}
This makes a cleaner source of adjusting the set of if criteria without having to dig through a long list AT the if statement itself, and you can re-use the same list as needed elsewhere with a single source to edit/change as needed.
Yeah, the above is just an example for me to explain a bit of what I'm doing. :) I apologize if I did not made myself clear.
As for your suggestion, I don't think I could use tags because there are situations that I require my gameobject to be two things at once, it could be an Oldman and an Enemy at the same time... and also I have already set raycast to detect gameobjects with a certain tag to identify it as selectedObject...
Now you mention I do realize that categorizing them has to be way, but is there a simple and smart way to categorize bools? I can only think of using one bool to check for other bools, could this be the only way? I feel it might cause me trouble to set up each scene.
Your answer
Follow this Question
Related Questions
Why does this text not equal the game object's name? 1 Answer
Add text on top of a gameobject. What is the best way? 1 Answer
Dynamically assign each ui.Text.text in list to the name of objects in another list 2 Answers
Update Text on Gameobject initiated from script from other Gameobject 0 Answers
Instantiate a GameObject at a position specific to an element that was found in a text 1 Answer