Bug - Fixed timesCast improperly tracked for Experience Safari (and presumably other player-targetted skills)

P

phreddrickk

Member
experience safari's timesCast property increments when it fails against a target (such as someone who is in hardcore).

> js Skill.get("safari") Returned: Experience Safari name => Experience Safari type => buff level => -1 image => jackmoustache.gif traincost => 0 class => none libram => false passive => false buff => true combat => false song => false expression => false walk => false summon => false permable => true dailylimit => -1 timescast => 0 > js useSkill(Skill.get("safari"), 1, "manendra") Casting Experience Safari on manendra 1 times... Selected target cannot receive buffs. Returned: false > js Skill.get("safari") Returned: Experience Safari name => Experience Safari type => buff level => -1 image => jackmoustache.gif traincost => 0 class => none libram => false passive => false buff => true combat => false song => false expression => false walk => false summon => false permable => true dailylimit => -1 timescast => 1

dailyLimit also seems to be inappropriately set, although I can just use the appropriate skillLevel for that.
 
H

heeheehee

Developer
Staff member
So, timescast is powered by SkillDatabase, which tracks these via registerCasts, which in turn is only called in FightRequest (in-combat) and UseSkillRequest (out-of-combat).

This is both much more pervasive yet not quite so pervasive as you might think -- a lot more cases are handled incorrectly here (e.g. original request was lost due to lag, not having the skill, not having enough MP), but some skill limits (e.g. for limited AT songs) are canonically tracked via preferences instead.

Ideally we'd either only increment skill usage on success, or roll it back on failure. There are a few revamps I'd like to make to that code, but I'm not feeling that ambitious at this time.

dailyLimit mismatch is a known and broadly unfixable issue if you used one or more of the skill-granting item or more before Mafia support was added, or when not logged into Mafia. If that's never happened, we want to know more about that! (Maybe KoL is giving us weird text that we're not processing correctly.) Otherwise, the workaround is as you've mentioned -- manually set skillLevel180 to the desired value.
 
P

phreddrickk

Member
heeheehee said:
dailyLimit mismatch is a known and broadly unfixable issue if you used one or more of the skill-granting item or more before Mafia support was added, or when not logged into Mafia. If that's never happened, we want to know more about that! (Maybe KoL is giving us weird text that we're not processing correctly.) Otherwise, the workaround is as you've mentioned -- manually set skillLevel180 to the desired value.
Click to expand...

I would say I'm 80% sure I used it while in mafia, but not 100%.

heeheehee said:
So, timescast is powered by SkillDatabase, which tracks these via registerCasts, which in turn is only called in FightRequest (in-combat) and UseSkillRequest (out-of-combat).

This is both much more pervasive yet not quite so pervasive as you might think -- a lot more cases are handled incorrectly here (e.g. original request was lost due to lag, not having the skill, not having enough MP), but some skill limits (e.g. for limited AT songs) are canonically tracked via preferences instead.

Ideally we'd either only increment skill usage on success, or roll it back on failure. There are a few revamps I'd like to make to that code, but I'm not feeling that ambitious at this time.
Click to expand...
Is there any workaround for this the way there is with the dailyLimit? What I'm trying to achieve is:
I have a list of safari targets
If I have safari casts, I iterate down the list, until I hit someone I can actually safari
then, I unload all my safaris against them.
 
H

heeheehee

Developer
Staff member
So, a terrible hacky workaround is to set skillLevel180 to a Very Large Number.

You probably don't want to do that, but it would technically work...

Maybe if I'm motivated this weekend I'll throw together a proper fix.
 
H

heeheehee

Developer
Staff member
Upon further reflection: we probably also do want to store data about how many casts you've already used, to support the case of someone logging in multiple times in one day.

The reported bug itself is a bit awkward to deal with because UseSkillRequest.parseResponse returns whether we should stop, while ideally we'd like to track whether it succeeded separately from whether to stop.

I assume that we never want to stop if the request succeeded, so really we've got three cases: SUCCESS, WARNING (failed, but not worth stopping over), and ERROR. (I have a fix that I've been tinkering with locally; probably won't send it for review until after the switch to github tomorrow.)
 
H

heeheehee

Developer
Staff member
Forgot to update this bug, but I think this should be fixed as of r25728. Let me know if it's still an issue.
 
P

phreddrickk

Member
Not entirely sure if this is in scope, but:

Code: 
>
> /cast 30 ancestral

Casting    Ancestral Recall 30 times.

You gain 3 Adventures
Preference    _ancestralRecallCasts changed from 0 to 10

> js    useSkill(1, Skill.get("safari"), "Captain Scotch")

Casting    Experience Safari on Captain Scotch 1 times...
Selected target cannot    receive buffs.
Returned: false

> /cast    safari @ captain scotch

Casting    Experience Safari on captain scotch.


>    /cast safari @ worthawholebean

Casting    Experience Safari on worthawholebean.

worthawholebean    (#1972588), the Level 46 Pastamancer
This player is currently    away from KoL in channel games and listening to hardcore and    trade.


> js useSkill(1,    Skill.get("safari"), "worthawholebean")

Casting    Experience Safari on worthawholebean 1 times...
Summon limit exceeded.
Returned:    false


Not sure the extent to which that stuff is in scope. So it successfully fails to count it when I use useSkill, but it doesn't track use when I do the chat macro through the CLI
 
H

heeheehee

Developer
Staff member
It... might be. I changed the results processing of UseSkillRequest only, but not anything to do with chat handling.

I think that's a separate issue where chat-based actions aren't properly registered. I suspect they weren't counted before my change, too.
 
H

heeheehee

Developer
Staff member
Hm. So, the logic I added does indeed correctly detect that I can't cast Ghostly Shell on Captain Scotch (and doesn't increment that skill's usage).

It doesn't print it to the gCLI, though.

Code: 
> /cast ghostly shell @ captain scotch

Casting Ghostly Shell on captain scotch.


> ash $skill[ghostly shell].timescast

Returned: 0

> /cast ghostly shell @ phreddrickk

Casting Ghostly Shell on phreddrickk.


> ash $skill[ghostly shell].timescast

Returned: 1

Might be nice to print the failure reason, but I think nominally the bug should be fixed.
 
You must log in or register to reply here.
Top