Computer's Decision is Sometimes nil in Battle [Solved]

Hello there again.

i am still working on my text-based RPG, and I am having another problem with it.

When I run the game, which currently consists of only one area (I have not programmed in the way the switch areas), things seem to run right, like I can view my equipment, stats, inventory, save, quit, and such, but while playing through the game, it sometimes crashes during battle, while saying that an unwrapped variable is nil, and it always occurs when it is the opponent's turn.

In the battle system, I have four things to choose from, which is attack, defend (does not do anything at this point), use an item, or flee, and I made it so that a random number determines what the enemy does, putting the bounds on the size of the array of choices, so there should never be a nil, yet one tends to come up.

Any ideas on what I need to do to fix this?

According to my thinking, the problem should be in the following function, which takes care of the enemy's turn.

func enemyTurn() {
        var choice = RandomGen(number: BattleEvent.allValues.count).number
        
        if (combatants[1].inventory.items.count == 0 && BattleEvent.allValues[choice!] == "item") {
            repeat {
                choice = RandomGen(number: BattleEvent.allValues.count).number
                
            } while (choice == BattleEvent.allValues.index(of: "item"))
            
            self.event = BattleEvent.allValues[choice!]
            
            self.outcome = self.action(source: combatants[1], target: combatants[0], choice: self.event, itemChoice: nil)
            
        } else if (combatants[1].inventory.items.count > 0 && BattleEvent.allValues[choice!] == "item")
        {
            var items: [String:Int] = [:]
            var itemOptions: [String] = [String]()
            
            for item in combatants[0].inventory.items {
                let keyExists = items[item.name] != nil
                if (!keyExists) {
                    items[item.name] = 1
                } else {
                    items[item.name] = items[item.name]!+1
                }
            }
            
            // loop that is only meant to grab key from dictionary
            for (item, _) in items {
                itemOptions.append(item)
            }
            
            let itemChoice = RandomGen(number: itemOptions.count).number
            
            self.event = BattleEvent.allValues[choice!]
            
            self.outcome = self.action(source: combatants[1], target: combatants[0], choice: self.event, itemChoice: combatants[1].inventory.items[combatants[1].inventory.items.index(where: {$0.name.caseInsensitiveCompare(itemOptions[itemChoice!]) == .orderedSame})!])
            
        } else {
            self.event = BattleEvent.allValues[choice!]
            
            self.outcome = self.action(source: combatants[1], target: combatants[0], choice: self.event, itemChoice: nil)
        }
    } // end function

However, I have tried a bunch of ideas, like checking if the random number is not nil or the array element is not nil, as well as using '??' to make '0' be default, but it tends to put out nil a lot.

If the character attacks, this function runs.

func attack(enemy: Character) -> Int {
        
        var dmg: Int = 0 // variable to hold damage
        
        if (RandomGen(number: 201).number > Int(enemy.evasion)) {
            dmg = self.attack
            
            if (self.gear.weapon != nil && self.gear.weapon.count != 0) {
                dmg += self.inventory.weapons[self.inventory.weapons.index(where: { $0.name.caseInsensitiveCompare(self.gear.weapon) == .orderedSame})!].damage
            }
            
            var cover = 0 // variable for enemy defense
            
            if (enemy.gear.head != nil && self.gear.head.count != 0) {
                cover += enemy.inventory.armor[enemy.inventory.armor.index(where: { $0.name.caseInsensitiveCompare(enemy.gear.head) == .orderedSame})!].defense
            }
            
            if (enemy.gear.torso != nil && self.gear.torso.count != 0) {
                cover += enemy.inventory.armor[enemy.inventory.armor.index(where: { $0.name.caseInsensitiveCompare(enemy.gear.torso) == .orderedSame})!].defense
            }
            
            if (enemy.gear.legs != nil && self.gear.legs.count != 0) {
                cover += enemy.inventory.armor[enemy.inventory.armor.index(where: { $0.name.caseInsensitiveCompare(enemy.gear.legs) == .orderedSame})!].defense
            }
            
            // create 1/32 change of a critical hit, ignoring defense
            if (RandomGen(number: 32).number == 0) {
                dmg = RandomGen(number: dmg*2).number
            } else {
               let baseDmg = dmg - cover/2
                dmg = RandomGen(number: baseDmg/4).number
                
                if (dmg < 1) {
                    dmg = RandomGen(number: 2).number
                }
            }
            enemy.hp -= dmg
        }
        return dmg
    } // end function

Since I am now not too sure where to look, here is the code that takes care of the battle.

func activate() -> Bool {
        
        // determine who goes first, player being highest priority
        print("HP: " + String(combatants[0].hp) + "/" + String(combatants[0].maxHp)) // make sure player HP is always displayed
        if (combatants[0].agility >= combatants[1].agility) {
            self.playerTurn() // initiate player turn
            
            // determine what player did and print message based on that action
            if (self.event == "attack") {
                if (self.outcome > 0) {
                    print(combatants[0].name + " dealt " + String(self.outcome) + " points of damage to " + combatants[1].name + "'s health.")
                } else {
                    print(combatants[0].name + "'s attack did not connect.")
                }
            } else if (self.event == "defend") {
                print(combatants[0].name + " decided to defend.")
            } else if (self.event == "item") {
                print(combatants[0].name + " healed themselves by " + String(self.outcome) + " points.")
            } else {
                if (self.outcome != 0) {
                    print(combatants[0].name + " fled from battle.")
                    return false
                } else {
                    print(combatants[0].name + " tried to flee from battle, but " + combatants[1].name + " prevented their escape.")
                }
            }
            
            // check if opponent is dead
            if (!combatants[1].isAlive()) {
                return false
            }

This function takes care of executing each choice:

func action(source: Character!, target: Character!, choice: String!, itemChoice: Item?) -> Int {
        if let selection = BattleEvent(rawValue: choice) {
            switch selection {
                case .attack: return source.attack(enemy: target)
                case .defend: return 0
                case .item: return source.useItem(item: itemChoice!)
                case .flee: return source.flee()
            }
        }
        return 0
    } // end function

Also, as I originally wrote this code on Linux, before being forced to test on a Mac environment. I had to create a custom class to deal with Random number generation, in order to cut down on instances of having to check for Linux, so here is the class for how my random number is being generated.

import Foundation

#if os(Linux)
    import Glibc
#else
    import Darwin
#endif

class RandomGen {
    var number: Int!
    init(number: Int) {
        #if os(Linux)
            self.number = random() % number
        #else
            self.number = Int(arc4random_uniform(UInt32(number)))
        #endif
    }
}

This should be all that should be needed, but in case it is not, I decided to break down and upload my code to Github, though I usually only put up stuff I got working there, and all the files can be found on this page.

Most of the code comes from the battle class and the attack function comes from the character class.

import Foundation

#if os(Linux)
import Glibc
#else
import Darwin
#endif

class RandomGen {
var number: Int!
init(number: Int) {
#if os(Linux)
self.number = random() % number
#else
self.number = Int(arc4random_uniform(UInt32(number)))
#endif
}
}

Why is RandomGen a class? I don’t see you ever keeping an instance of the class. Why not a standalone function? That also gets rid of an optional -- although, I’m not sure why it was a IUO in the first place.

// return a random integer value between 0 and the given limit
func randomGen(limit: Int) -> Int {
#if os(Linux)
return random() % limit
#else
return Int(arc4random_uniform(UInt32(limit)))
#endif
}

Don’t know if that has anything to do with your problem, though.

I utilize the function in multiple instances. One in the main swift file, dealing with outcomes of searching, another in the character class, for things like leveling up, dealing with attacks, and fleeing, in addition to AI's decision's in battle, much of which is in seperate files, so I had it in my mind that I needed in something like a struct or class (made class out of habit from coding for Android/Java).

The code itself is in an individual file, and from looking around, it looks like I would still need to do something like class/struct initialization, which is filename().function(parameters), but it does look possible to do what you are talking about.

Of course, the random number determining outcome of searches is never nil, so I do not think it will do anything, but I can give it a try on an experimental branch.

Update: I just tried converting RandomGen to a function, with no modifications, aside from removing the property "number" and removing the markers for unwrapping, and RandomGen still worked (meaning functions can indeed be in files of their own without any problems and not needed an instance of the file be declared).

However, I still got a message about an unexpected nil while it unwrapped, so there was no change.

I'm not sure what your exact problem is, but looking at the code I can see a few structural issues that won't be helping in preventing bugs. For example:

func action(source: Character!, target: Character!, choice: String!, itemChoice: Item?) -> Int {
        if let selection = BattleEvent(rawValue: choice) {
            switch selection {
                case .attack: return source.attack(enemy: target)
                case .defend: return 0
                case .item: return source.useItem(item: itemChoice!)
                case .flee: return source.flee()
            }
        }
        return 0
    }

Having a function take implicitly unwrapped optionals as arguments seems a very bad idea to me. You should know at the point of the function call whether the arguments exist or not. If you can't say for sure that source or target won't be nil, then you're very likely to crash at runtime.
As another example, you have an implicit assumption that itemChoice will not be nil if choice is .item; have you already determined at the time you call this function that choice is .item, and if so made sure that itemChoice is not nil? If you already know that, it seems odd to me to go back through this function where you're throwing away that information.

I'd strongly suggest you try to restructure this code to reduce the number of Optionals and rely more heavily on the type system. In particular, get rid of implicitly unwrapped optionals - having Int! properties on a struct or class is just asking for trouble except in rare circumstances (lazy initialisation or circular references) which I don't think apply for you.

For example, in lines like:

enemy.inventory.armor[enemy.inventory.armor.index(where: { $0.name.caseInsensitiveCompare(enemy.gear.torso) == .orderedSame})!].defense

Presumably here enemy.gear.torso is a String, while the inventory.armor array contains the classes with the actual information. Why is enemy.gear.torso not a reference to that same piece of data? I can see that you're loading from a json file, but I'd personally use the text in the JSON to connect up references to the actual objects when you load.

As another style thing:

 if (enemy.gear.torso != nil && self.gear.torso.count != 0) {

can be reformulated as:

if let enemyTorso = enemy.gear.torso, !self.gear.torso.isEmpty {

which makes enemyTorso non-optional within the scope of that if statement.

In short, I'd suggest going through your code, finding everywhere there's an ! and removing it. Then, when there are cases where you need to force unwrap things (e.g. loading data from disk where it's an error if the data format doesn't match what you expect), force unwrap them as early as possible so you discover the error early.

1 Like

In my code, I have the classes set up as conforming to Codable, as opposed to separate instances for Encodable and Decodable, so it will keep making objects when character is written to JSON or loaded.

I used String, in order trim that down, so instead of having two identical objects listed for each character, there would be one, with Strings dictating what is equipped, and I would to grab it via the String name.

Could happen with enemies and areas as well, but it will still result in the same occurrence of Strings, when I need an object.

I will try to look into what you are advising for most of it. The Most difficult will be removing the forced unwrapping on that array with string comparisons, because every time I try removing it the compiler keeps complaining, so I need to either put it back in or use β€˜??’ to set a default (cannot really get a good example of subscripts), as it is try to retrieve the object of that name from an array.

The others sound simple enough to look into.

In my code, I have the classes set up as conforming to Codable, as opposed to separate instances for Encodable and Decodable, so it will keep making objects when character is written to JSON or loaded.

What I'd suggest here is to separate your code into the persistent objects or types – things that can't change between save files – and temporary state. For example, a particular piece of armour might have certain stats, and you have a class or struct representing that piece of armour; that shouldn't change between loading or saving the game. However, whether the character has a copy of that armour in their inventory may change, so what you'd save out is the player's inventory with a reference to that particular piece of armour (for your case it could just be a String, but often in games you'd have some UUID-type identifier that uniquely represents that item).

Then, in your loading of the player, you could do something like:

for itemIdentifier in savedInventory {
    let item = ItemRegistry.item(identifier: itemIdentifier)
    player.inventory.add(item)
}
1 Like

For things like this its okay to use force unwrapping if you know what you're looking for will always be there. If this isn't guaranteed you might consider using an if-let construct:

if let idx = inventory.weapons.index(where: {$0.name.caseInsensitiveCompare(gear.weapon) == .orderedSame}) {
  dmg += inventory.weapons[idx].damage
} else {
  // handle error for a weapon not in inventory.
}

It seems like you're using a lot of Java patterns in your code and from what you mention it is because that's where you're coming from. It's great to see that you're giving Swift a chance, but if you really want to understand why so many of us love Swift I encourage you to definitely try the some of the things people above are suggesting above :)

Some other things to consider:

  • Not using self before accesses to items (unless it's helpful for you to understand)
  • Reduce expression complexity. I'll use an example from your code in file "menu.swift", function viewArmor. Where you "add dictionary items to string variable for output", you might consider rewriting it as follows:
let desc = assets.player.inventory.armor.first {
    $0.name.caseInsensitiveCompare(armor) == .orderedSame
}?.report ?? "No report available"

let defense = assets.player.inventory.armor.first {
    $0.name.caseInsensitiveCompare(armor) == .orderedSame 
}?.defense ?? 0

let slot = assets.player.inventory.armor.first {
    $0.name.caseInsensitiveCompare(armor) == .orderedSame
}?.slot ?? "Slot unavailable"

display += """
\(armor): \(total)

Description: \(desc)
Defense: \(defense)
Slot: \(slot)
"""

It occurs to me that a lot of your code has a lot of boilerplate in the form of member lookups. A with statement would be quite useful for these cases, however, Swift doesn't have them explicitly, but you can use anonymous closures instead. The example above could possibly be changed to the following:

let (desc, defense, slot): (String, Int, String) = { inv in
    let desc = inv.armor.first {
        $0.name.caseInsensitiveCompare(armor) == .orderedSame
    }?.report ?? "No report available"

    let defense = inv.armor.first {
        $0.name.caseInsensitiveCompare(armor) == .orderedSame 
    }?.defense ?? 0

    let slot = inv.armor.first {
        $0.name.caseInsensitiveCompare(armor) == .orderedSame
    }?.slot ?? "Slot unavailable"
    
    return (desc, defense, slot)
} (assets.player.inventory)

I am currently looking at the code, and things have not been fixed yet (I am incorporating the some of the suggestions, aside from the random number one, which I already put in place after finding out functions do not need to be wrapped in anything to be accessible from other files around the time I updated one of my replies), and I noticed that I was referring to the character's own gear a lot, instead of the enemy's, so I am also fixing that in addition to trying to figure out other things.

Hopefully, that will help fix things.

Some of the other stuff will take a bit more searching/experimentation, but I appreciate how you guys are making things simple to understand.

Update: I deleted my original experimenting branch with the changes that you guys were suggesting, and created a new one around the time I originally posted this, though I did change a few things, such as using if lets with variable name as '_' (some will definitely need to be reworked to get things right, because compiler will complain if I ommit '!' or '?' from variable declaration, even if a function called in the init statement does give them values.

After changing the typos of 'self.gear' to 'enemy.gear', things started to look right, so it looks like that was causing the nil. As such, I will be marking this as solved, but will also be bookmarking this question, as I was getting useful advice, especially for things that I kind of wanted to do eventually (e.g. Item objects being housed somewhere other than with the JSON for areas and characters, so I modify items in only one file or place).