As some students may have noticed, there is a slight flaw in the ointment when it comes to using Abilities in the Shops and Abilities course. Actually… there are two flaws
- If there are no valid targets selected by the targetingstrategy, the cooldown and mana are still spent.
- If you can’t actually use an ability due to lack of Mana, cooldown, or a lack of valid targets, but the item is consumable, then the item still disappears from inventory!
It’s easy not to notice these things in playtesting, but they are certainly not issues you want to carry into your production.
The tricky part is the way we implement the abilities. The actual Use method calls a Targeting Strategy, which then returns in Targeting Finished, then applying filter strategies and finally effect strategies. Where do we actually consume the three consumable resources (mana, cooldown, and the item itself)?
Let’s start with dealing with the first issue… if there are no valid targets selected by the TargetingStrategy, the cooldown and mana are still spent.
This has to do with the order in which we are doing things in the TargetAquired method:
private void TargetAquired(AbilityData data)
{
if (data.IsCancelled()) return;
Mana mana = data.GetUser().GetComponent<Mana>();
if (!mana.UseMana(manaCost)) return;
CooldownStore cooldownStore = data.GetUser().GetComponent<CooldownStore>();
cooldownStore.StartCooldown(this, cooldownTime);
foreach (var filterStrategy in filterStrategies)
{
data.SetTargets(filterStrategy.Filter(data.GetTargets()));
}
foreach (var effect in effectStrategies)
{
effect.StartEffect(data, EffectFinished);
}
}
Note that we first check to see if the data is cancelled… Then we cook off the mana and set the cooldown. Finally, we filter. Now suppose you are casting a healing spell, but your character is already at full health. You would be wasting that healing spell altogether. Similarly, if you cast a spell to burn all characters within a target circle, but there are no characters, you probably don’t want to cast the spell…
So we can do a little re-ordering and use if conditions to avoid this.
private void TargetAquired(AbilityData data)
{
if (data.IsCancelled()) return;
foreach (var filterStrategy in filterStrategies) data.SetTargets(filterStrategy.Filter(data.GetTargets()));
if(data.GetTargets().Any())
{
var mana = data.GetUser().GetComponent<Mana>();
if (!mana.UseMana(manaCost)) return;
var cooldownStore = data.GetUser().GetComponent<CooldownStore>();
cooldownStore.StartCooldown(this, cooldownTime);
foreach (var effect in effectStrategies) effect.StartEffect(data, EffectFinished);
}
}
Now, if there are no targets, nothing happens…
Of course, this doesn’t work if our targeting strategy is DirectionalTargetting like the fireball that shoots at the location of the mouse… because we don’t actually get a target, we just fire in a certain direction.
In this case, you might not want to short circuit the operation of the spell just because of no targets…
For this, we can add a Serialized Field to the Ability class
[SerializeField] private bool noTargetRequired = false;
Now we can change the if statement to read
if(noTargetRequired || data.GetTargets().Any())
Now for spells like the Directional Fireball, you can set noTargetRequired and there won’t even be a check to see if there are any targets for the spell to act on.
So this takes care of the first issue… now to tackle the harder issue, not consuming an item if we can’t actually use the spell at this time…
Currently, our AbilityItem.Use() spell has one parameter, the calling GameObject, and the AbilityStore Use method simply checks to see if the item is consumable and removes the item if it is upon Use(). But the truth is we don’t know if we’ll consume the item until we’ve gotten through the Targeting and possibly filtering stage. We haven’t even checked the Mana and Cooldown, we just deduct the item… So we might be firing off fireballs at all the enemies, but not noticing that because the first fireball’s cooldown hasn’t finished, we can’t actually call the next. The player spams the keyboard and bewm, his wand of fireballs is empty, and he’s only fired off 1 or 2… where did the other 10 go?
This solution is a bit more complicated, and it’s going to rely on a trick we learned in the Shops section, the lambda expression.
All of the following changes will need to be made for the code to compile, so bare with me…
First… we need to have a way of saying to the ActionStore “Hey, we’ve actually used this item”. The trouble is we don’t know if that message is going to come right away or five minutes from now when the user finally clicks on his DelayedClickTargeting (or ever, if he cancels the ability!)
So we need to have some sort of callback… Fortunately, System.Actions are great for just this thing…
So we’ll start with a System.Action completedCallback.
Now if the item isn’t consumable, then there’s really no reason to have a callback, but if it is consumable, then we absolutely need one… And what we want that callback to do is what we’re already doing in the original Use code:
Here’s the original Use method in ActionStore for reference:
public bool Use(int index, GameObject user)
{
if (dockedItems.ContainsKey(index))
{
dockedItems[index].item.Use(user);
if (dockedItems[index].item.isConsumable())
{
RemoveItems(index, 1);
}
return true;
}
return false;
}
So in this case, we’re simply removing the item right away if it exists within the ActionStore, not even considering whether or not it’s actually been sucessfully used… let’s change the way this is managed…
Here’s my modified Use method:
public bool Use(int index, GameObject user)
{
if (dockedItems.ContainsKey(index))
{
System.Action callback = () =>
{
RemoveItems(index, 1);
};
dockedItems[index].item.Use(user, dockedItems[index].item.isConsumable()?callback : null);
return true;
}
return false;
}
So what we’ve done here is create a System.Action which is a Lambda expression (remember we used Lambda expressions in the Shops section when we assigned the buttons. Our expression contains one simple statement,
RemoveItems(index, 1);
Now when this callback is created, an anonymous function is essentially declared with the ()=>, and the status of the variables in this function are locked into place… So even if we later called this method with a different index, it doesn’t matter, because for this anonymous function, the index is set as the index passed into the method at this time. This is commonly called a Closure.
At this point, however, your compiler is likely screaming at you that no method in ActionItem takes in a System.Action… so that brings us to the next change, in ActionItem.cs
public virtual void Use(GameObject user, System.Action consumeCallback)
{
Debug.Log("Using action: " + this);
consumeCallback?.Invoke();
}
We’ve simply added the consumeCallback to the method declaration. This virtual method should always be overridden, but there’s a simple framework for testing.
And now our compiler should be complaining about Ability.cs not having a suitable method to override, because the signature doesn’t match.
That’s the next change, adding the System.Action consumecallback to the Use declaration
public override void Use(GameObject user, System.Action consumeCallback)
So now we have the link to our consumeCallback, and technically speaking our code will compile, but we still haven’t actually called the consumeCallback yet…
We can’t really call it in Use, because in Use itself we actually have no idea if we’re going to really use the ability or not, until we get through the Targeting phase and Filtering phase. So we’re going to have to store that callback somewhere.
Remember that since our abililities are ScriptableObjects, we can’t actually store the callback there, because if we eventually have our enemies use abilities, they will be using the same ScriptableObjects. Therefore, we need to put the callback in our User data.
Head into AbilityData, and add these lines
private System.Action consumedCallback; //Edit: typo in first draft said abilityCallback
public void SetConsumedCallback(System.Action callback)
{
consumedCallback = callback;
}
public void InvokeConsumedCallback()
{
consumedCallback?.Invoke();
}
The first method assigns the callback. The second method invokes the callback if it is not null.
Now in our AbilityItem.Use() method, we can set the consumedCallback on the AbilityData (after creating the new AbilityData, of course!
data.SetConsumedCallback(consumeCallback);
Now that our AbilityData has a copy of the callback, we can continue on with our Targeting and Effect strategies… let’s go back to the TargetAquired method and put the final touches on our fix:
private void TargetAquired(AbilityData data)
{
if (data.IsCancelled()) return;
foreach (var filterStrategy in filterStrategies) data.SetTargets(filterStrategy.Filter(data.GetTargets()));
if(noTargetRequired || data.GetTargets().Any())
{
var mana = data.GetUser().GetComponent<Mana>();
if (!mana.UseMana(manaCost)) return;
data.InvokeConsumedCallback();
var cooldownStore = data.GetUser().GetComponent<CooldownStore>();
cooldownStore.StartCooldown(this, cooldownTime);
foreach (var effect in effectStrategies) effect.StartEffect(data, EffectFinished);
}
}
Now we should have patched both of these inconsistencies in the Abilities.
If there is no target (when a target is required), the ability will not use Mana or start the Cooldown. Consumable abilities will only be consumed when the Mana and Cooldown are actually set, not at the attempt to activate the ability.