if statement logic

Hi, I think I’ve worked for too long and my brain is fried cause I can’t figure this out.

The logic in my if statement is not working. Particularly the else if at the bottom of the code with lot’s of criteria it executes even if the first if in that statement was completed.
To give you some context: a 2D tilemap world with grass, forests and rivers. This script decides which bridge tile to use: horizontalBridgeTilePrep or verticalBridgeTilePrep - when placing the building, it’s green and it follows the mouse on the tilemap grid till the building is placed down
horizontalBridgeRed - same as before but it’s red signaling that the location is invalid

(The location can be valid if we are trying to place down the bridge on water and if left and right tiles are Grass or Forest then place down horizontal bridge(which means the river flows vertically so the bridge is horizontal). as for vertical bridge it’s the same just rotated…)

The issue: both the horizontal and vertical bridges work great, it’s just that the else if code executes right after the horizontal if statement and overrides the horizontalBridgeTilePrep to horizontalBridgeRed (So it seem like the location is invalid, but it’s not(I’ve made a debug.log that tells prints out all the tiles that are around the place where I’m trying to place the building and it fit’s I’ve also tested it with other buildings works great))
I’ve placed in 2 other debug.logs GREEN and RED and both of them execute(When I’m trying to place down a horizontal bridge). First GREEN and then RED right after that.

if (buildingName == "Bridge" && canPlace == true)
        {
            if (current == "Water" && (left == "Grass" || left == "Forest") && (right == "Grass" || right == "Forest") && currentL1 == "Nothing")
            {
                buildingRenderer.SetPrepTileTo(mousePos, horizontalBridgeTilePrep); //Set visuals of the GREEN prep building 1 layer above buildings layer
                Debug.Log("GREEN");
                if (Input.GetMouseButtonDown(0))
                {
                    buildingRenderer.SetTileTo(mousePos, horizontalBridgeTileConstruction); //Set Visuals
                    SetTileGridBuildings(mousePos, nameof(horizontalBridgeTileConstruction)); //Set backend grid value
                    canPlace = temp; //Set it back to false
                }
            }
            if (current == "Water" && (up == "Grass" || up == "Forest") && (down == "Grass" || down == "Forest") && currentL1 == "Nothing")
            {
                buildingRenderer.SetPrepTileTo(mousePos, verticalBridgeTilePrep); //Set visuals of the GREEN prep building 1 layer above buildings layer
                if (Input.GetMouseButtonDown(0))
                {
                    buildingRenderer.SetTileTo(mousePos, verticalBridgeTileConstruction); //Set Visuals
                    SetTileGridBuildings(mousePos, nameof(verticalBridgeTileConstruction)); //Set backend grid value
                    canPlace = temp; //Set it back to false
                }
            }
            else if (current != "Water" || currentL1 != "Nothing" || (current == "Water" && (
                (up != "Grass" || up != "Forest") && (down != "Grass" || down != "Forest") && (left != "Grass" || left != "Forest") ||
                (up != "Grass" || up != "Forest") && (down != "Grass" || down != "Forest") && (right != "Grass" || right != "Forest") ||
                (left != "Grass" || left != "Forest") && (right != "Grass" || right != "Forest") && (up != "Grass" || up != "Forest") ||
                (left != "Grass" || left != "Forest") && (right != "Grass" || right != "Forest") && (down != "Grass" || down != "Forest")
                )))
            {
                buildingRenderer.SetPrepTileTo(mousePos, horizontalBridgeRed); //Set visuals of the RED prep building 1 layer above buildings layer
                Debug.Log("RED");
            }
        }
        else temp = false;

Simplify, simplify, simplify. Those if statements are gobsmackingly complicated and even without you saying so I would be near certain that a mistake was in there somewhere. The way advanced programmers avoid problems with big if statements isn’t by getting better at parsing big if statements; it’s by knowing how to make a design that doesn’t use big if statements.

And imagine if next week, you need to add a “Dirt” tile, and figure out which 36 places it needs to get inserted into that code. Or a “Canyon” tile over which you also want roads to be buildable. The nightmare gets exponentially more complicated with every change.

Rather than marking each tile with a string and running logic on every string, create a data structure that includes these types, and use that data to make these decisions. If your originating data (not posted here) is already committed to being strings, you could put this data in a dictionary and do a lookup.
So for example:

public class TileData {
   public string tileName;
   public bool canBuildOn = false;
   public bool canBuildNextTo = false;
   public bool canBuildBridgeOver = false;
}

public Dictionary<string, TileData> tileLookup = new Dictionary<string, TileData>();

void Awake() {
tileLookup.Add("Water", new TileData() { tileName = "Water", canBuildOn = false, canBuildNextTo = true, canBuildBridgeOver = true });
tileLookup.Add("Grass", new TileData() { tileName = "Grass", canBuildNextTo = false, canBuildOn = true });
}

TileData up = tileLookup[whatever]; //use this in place of your current "up" string variable
//etc
if (current.canBuildOn ) {

Obviously I don’t know what attributes your different tile types have on them (and more relevantly, you probably don’t know what tile types you will have 2 weeks from now), but this should serve as a model. It will allow you simplify your if statements by a couple of orders of magnitude, and I’m guessing in the process, issues with your logic will reveal themselves.

Anyway, aside from all that, I think line 14 is probably supposed to be an “else if”. :wink:

4 Likes

Thanks you. That else if in the middle fixed it all (I can even drop the huge condition line at the end if I just change the else if to else) I’ve used other programing languages and I’ve always used:
if()
if()

else
(but that didn’t work now for some reason previously unknown)

Now I’ve watched a unity c# tutorial on the if else statements and figured out that it should be:
if()
else if()

else()

Ok next up is learning data structures. Thanks

I’ve been struggling with this data structure thing for hours :frowning:

I have 2 scripts: Buildings and TileData
(I’ve deleted parts of the scripts which were not relevant)

I’m calling the lookup from Buildings to TileData through: tileData.TileDataLookUP(up, out tUp);
and I want it to return the TileData struct so I can use it in my code like: if (current.canBuildOn ) { and so on.

But I’m getting an error at(tUp = tileLookup[“Grass”]; in the second script):
KeyNotFoundException: The given key was not present in the dictionary.
System.Collections.Generic.Dictionary`2[TKey,TValue].get_Item (TKey key) (at <695d1cc93cca45069c528c15c9fdd749>:0)
TileData.TileDataLookUP (System.String up, TileData& tUp)

It’s not present does that mean it wasn’t created? or what am I doing wrong?

public class Buildings : MonoBehaviour
{
    public TileData tileData = new TileData();

void Update()
    {
            TileData tUp
            Debug.Log(up);
            tileData.TileDataLookUP(up, out tUp);
    }
}
public class TileData
{
    public Dictionary<string, TileData> tileLookup = new Dictionary<string, TileData>();
    public string tileName;
    public bool canBuildOn = false;
    public bool cantBuildNextTo = false;
    public bool cantBuildNextToOnHill = false;
    public bool canBuildBridgeOver = false; //For bridge and fishing
    public bool canBuildBridgeOverNextTo = false; //For bridge and fishing

    void Awake()
    {

        tileLookup.Add("Water", new TileData() { tileName = "Water", canBuildOn = false, cantBuildNextTo = true, canBuildBridgeOverNextTo = false, canBuildBridgeOver = true });
        tileLookup.Add("Grass", new TileData() { tileName = "Grass", cantBuildNextTo = true, canBuildOn = true, cantBuildNextToOnHill = false, canBuildBridgeOverNextTo = true });
        tileLookup.Add("Forest", new TileData() { tileName = "Forest", cantBuildNextTo = true, canBuildOn = false, cantBuildNextToOnHill = false, canBuildBridgeOverNextTo = true });

    }

    public void TileDataLookUP(string up, out TileData tUp){
        //tUp = tileLookup[up];
        tUp = tileLookup["Grass"];
    }
}

Nevermind I’ve fixed it.