20110803

Resolving Dependencies When Loading

Boring title, I know. Boring post follows. I ran across a problem, spent too long trying weird, overwrought solutions, then realized the simple answer had been staring at me all along. As always, I write this to capture the thought process.

The problem (at the highest level): when someone tries to set the current HP of a mob (any creature in the game) to be greater than its max HP, we should cap it, so that they don't get super powers all of a sudden.

As an aside, there are MUDs where the current HP can soar higher than the max HP, but only for a short time. That's a policy I can debate about internally anytime, but assuming I do want to enforce this, I should have a mechanism to do so.

Here's the code where I put it:

class Mob
...
    def currentHP=(newHP)
        # the else case for Numeric is used during loading
        if newHP.kind_of?(Numeric)
            if newHP < 0
                newHP = 0
            elsif newHP < maxHP()
                newHP = maxHP()
            end

            @currentHP = newHP

            if currentHP() == 0
                die()
            end
        else
            @currentHP = newHP
        end
    end # function currentHP=
end # class Mob

Pretty straightforward, at least at a glance. Of course, it's not so. Time to think back again to loading and saving, those eternal thorns in my side. Quick refresher: when we load up the MUD, we parse a bunch of XML files and create a bunch of objects out of what's described there. The objects are things like Mobs or Rooms or Dealies (objects). Each object can have properties. For example, a Mob has a property called current_hp:

# a numeric property loaded from the xpath "current_hp", which stores its value
# in self.currentHP, and if not found, takes its value from self.maxHP

addProp(
    PersistedProperty.new(
        'current_hp',
        ref(:currentHP),
        Fixnum,
        PersistedProperty::OptionalValue.new(ref(:maxHP))))

What happens during loading is that all the properties of an object are loaded in an unspecified order. To set the property, it calls the accessor method on the object, like Mob.currentHP=. Due to the undefined order, there's a nonzero chance that loading will try to set the property current_hp before max_hp. You can see from the code snippet before that currentHP depends on max HP for the capping logic, so it fails.

What to do? What to do? I tried two things before arriving at my solution.

Try 1: explicit dependency annotations

I said to myself, "Well, I have an ordering problem. I need to instruct the loading system not to try to set current_hp first." So I did exactly that. I added more data to PersistedProperty to check its dependencies before attempting to get or set it. Pretty straightforward stuff. NotLoaded is not a new thing; it is used in the loading system to indicate that a property has not yet been loaded. It's better than using nil, too.

##########
# if we have dependencies on other things, check all of them first. only if
# they are all loaded can we set or return our real value
##########
def getProp()
    if hasDependenciesFulfilled?()
        return @refProp[]
    else
        return PartiallyLoadable::NotLoaded
    end
end # function getProp

# setter omitted for brevity

def hasDependenciesFulfilled?()
    if dependsOn()
        dependsOn().each() { |dep|
            if dep[] == PartiallyLoadable::NotLoaded
                return false
            end
        }
    end

    return true
end # end function hasDependenciesFulfilled?

addProp(
    PersistedProperty.new(
        'current_hp',
        ref(:currentHP),
        Fixnum,
        PersistedProperty::OptionalValue.new(ref(:maxHP)),
        nil,
        [ref(:maxHP)]))

In the addProp call we simply make sure to indicate that current_hp depends on max_hp, and all is well! Modulo some problems with protoRefs that I won't get into now, it actually does work just fine. But I don't like this design.

Now there are two places that know about this dependency: Mob.currentHP=, and this place where we declare the dependency. Generally speaking, it's best not to have duplicate logic spread out like this. If current_hp suddenly doesn't depend on max_hp anymore, now two places need to be updated to be in sync. And it's really easy to forget one of them, since it's kind of off to the side.

So I add another goal: only one place should know that this dependency relationship exists.

Try 2: raise exception when failing to set due to dependency

I thought: "maybe if currentHP= fails, I should error out or raise some kind of exception." The loading system can catch and expect that, and come back to the property later, when the dependency has been fulfilled.

So I started writing up a little mechanism using throw and catch, which actually works kind of differently than in other languages. In Ruby it is a general purpose mechanism (not specialized for error handling at all) to unwind the stack.

You write a catch block, which returns a value: either the last thing evaluated in the block--just like a function--or the value passed to a throw call somewhere underneath. If you've been keeping up (*guffaw*) you may remember that I already used this mechanism in my Mobchecks code.

I started implementing this, but after thinking about it a little, decided not to. Taking a step back, the desire is to give the loading code a way to recognize the failure to set a property... but to what end? Well, if it notices a failure, it will know to try again later. Actually, this sounds pretty familiar...

Try 3: setter silently fails due to dependency

It turns out the loading code is not very performant and kind of brute-force right now. It already has this similar problem in a lot of places. The way it handles it is by checking after setting a property, to see if it "stuck".

# trying to set an unloaded property to a real value
if (NotLoaded != propVal) && (NotLoaded == prop.getProp())
    prop.setProp(propVal)

    ##########
    # sometimes we try to set the prop, but it can't really be set,
    # so here we check whether it "stuck"
    ##########
    propVal = prop.getProp()

    if NotLoaded == propVal
        DebugOutput.debugOut(LComment | LLoading){"prop #{prop} was STILL not loaded"}
        everythingLoaded = false
    end
end

So it already has a way of detecting if it failed to set a property. And so we can simply have currentHP= silently not set the value. And this works equally well for any dependency situation like this. The setter checks to see if it has enough info to be set. If not, it just doesn't, assuming later on it will be called again under circumstances where it will work. A simple solution that doesn't introduce any weird new mechanisms. That is really the best, isn't it?

0 comments:

Post a Comment