[Combat] Implement defaultIfNil utility function

[Xaver-DaRed]

I affirm:

• I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
• I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
• I have read and understood the Contributing Guide and the Code of Conduct.
• I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Implements an utility function which will default an unknown value to a known one, if the original value is nil, and will also report the defaulting.

Steps to test these changes

None.

[zach2good]

These will produce errors if actorLevel or skillRank are passed in as nil.

Some test code:

local actorLevel = nil
local skillRank = nil

-- Sanitize fed values
local levelToCheck = actorLevel >= 0 and actorLevel or 0           -- Assume level 0
local rankToCheck  = skillRank > 0 and skillRank or 0 -- Assume rank G

print(levelToCheck, rankToCheck)

lua5.4: Main.lua:5: attempt to compare number with nil
stack traceback:
	Main.lua:5: in main chunk
	[C]: in ?

They need to be in this order to be safe:

-- Sanitize fed values
local levelToCheck = actorLevel and actorLevel >= 0 or 0 -- Assume level 0
local rankToCheck  = skillRank and skillRank > 0 or 0 -- Assume rank G

[Xaver-DaRed]

Pushed changes and tested properly

[zach2good]

These change's aren't what I meant:

    local levelToCheck = (actorLevel and actorLevel > 0) and actorLevel or 0           -- Assume level 0
    local rankToCheck  = (skillRank and skillRank > 0) and skillRank or xi.skillRank.G -- Assume rank G

You're checking the same condition twice.

It's better to take the example I gave:

-- Sanitize fed values
local levelToCheck = actorLevel and actorLevel >= 0 or 0 -- Assume level 0
local rankToCheck  = skillRank and skillRank > 0 or 0 -- Assume rank G

This pattern of nil checking with a default option is used in a bunch of places and it relies on short-circuit evaluation to work:

local thing = maybeNil and maybeNil > 0 or 0

If maybeNil is nil the rest of the and statement won't be evaluated and the other side of the or is taken - the same with maybeNil > 0

[zach2good]

I'd avoid adding any ()'s if it isn't needed

[Xaver-DaRed]

Using this code

-- Sanitize fed values
local levelToCheck = actorLevel and actorLevel >= 0 or 0 -- Assume level 0
local rankToCheck  = skillRank and skillRank > 0 or 0 -- Assume rank G

Would populate both variables (levelToCheck and rankToCheck) with a true value (or a 0 if nil)
In c++ it would be the same as doing this:
levelToCheck = actorLevel ? actorLevel >= 0 : 0
rankToCheck = skillRank ? skillRank >= 0 : 0

[zach2good]

You are 100% correct and I'm talking out of my ass.

A helper like this in utils would make this sort of thing a lot more readable:

function defaultIfNil(inputValue, defaultValue)
    if inputValue == nil then
        local info = debug.getinfo(2, 'Sl')
        print(string.format("nil value encounted at %s:%i, defaulting to %i", info.source, info.currentline, defaultValue))
        return defaultValue
    end
    return inputValue
end

local val = nil

local checkedVal = defaultIfNil(val, 0)

function(input)
    -- validation
    input = utils.defaultIfNil(input, 0)
    input = utils.clamp(input, min, max)

    -- now we can do maths

    -- clamp and floor on the way out, if needed
end

[zach2good]

valueIfNil or defaultIfNil is better now that I see it, sorry for the runaround