ID:2136874
 
(See the best response by Ter13.)
Code:
Click()
sleep(1)
if(!usr.Attempt&&!src.Active)
usr.Attempt=1
src.Active=1

Problem description:Double Casting when using \n macros

basically i have a setup with obj based skill, clicking them sets them off.
These techs all have a cool down after usage

I'm having an issue where ".click TechName\n.click TechName" can cause the tech to be used multiple times before actually bringing in the cool down.

what I've done to this point, was give a variable that is triggered on usage to both the obj and the usr without any success whatsoever.

any help would be greatly appreciated


That sleep(1) at the beginning is what's preventing the usr.Attempt and src.Active checks from working in a timely manner.

You can also prevent the usage of .click by enabling client.control_freak.
To expand on what Nadrew said, you have to sleep after you begin the cooldown period, not before, and be sure to reset the vars when the cooldown is finished. If you sleep before setting the vars, then during that sleep time anything can come along and call this again.

Click()
if(usr.Attempt || Active) return
usr.Attempt = 1
Active = 1
DoSkill(usr) // do whatever this skill does
sleep(cooldown)
usr.Attempt = 0
Active = 0
Sorry let me expand a bit, I added the sleep in one of my attempts to fix it. It does it with or without it
I have a cool down setup down and working, but the issue I'm having the second or third technique makes it past th

if(usr.Attempt || Active) return


Before it is triggered as currently active, this same thing doesnt happen if it's a single macro only happens with any \n macro

Edit, I'd just like to add I'd prefer to keep .click usable by players
What does your code look like now?
Dumbed down version of what I just tried to get around it, I realize I shouldn't be changing much when trying to get help with fixing something so i apologize in advance for that.

This is a dumbed down version of the entire proc, but its more or less the idea of it. Ignore any typos you may see here I'm just trying to get across the idea of how I have it set up

    Click()
sleep()
Active++
if(!usr.Attempt)
usr.Attempt=1
var/mob/living/player/U=usr
if(Active>1)
Active--
return
if(U.cooldown["[Sindex]"]<=0&&Active==1)
if(U.Mana.Current>=MCost&&U.Stamina.Current>=SCost&&Active==1)
if(MCost)
U.Mana.StatDeduct(MCost,usr,usr)
if(SCost)
U.Stamina.StatDeduct(SCost,usr,usr)
U.Skill(Sindex,src)
usr.JAttempt=0
U.cooldown["[Sindex]"]+=30
else
if(U.Mana.Current<MCost)
usr<<"Not enough Mana[round(U.Mana.Current,1)]/[round(MCost,1)]"
if(U.Stamina.Current<SCost)
usr<<"Not enough Stamina [round(U.Stamina.Current,1)]/[round(SCost,1)]"
usr.JAttempt=0
Active=0
else
var/r=U.cooldown["[Sindex]"]
usr<<"Cooling Down ([r] seconds left)"
E.JAttempt=0
Active=0


In this instance, Sindex would be a number representing which technique it is. while cooldown is a list relating back to that sindex number
U.Skill() probably has sleeps in it, doesn't it?

You want to increase the skill cooldown BEFORE you call any proc that will sleep, or you want to set the Skill() function to waitfor = 0 to prevent the function from stopping the downstream cooldown increase.

Basically, if U.Skill() has sleep() anywhere in it, it will prevent the cooldown from being added until U.Skill() is done.

Also, get rid of that sleep() at the top of Click(). It's not needed.
U.Skill() is already set to waitfor 0 for that exact reason.

Also sleep now removed thanks ;)

ignoring cooldown isnt really my issue in this case.
In reality right now the issue as i see it, is that its getting past these
if(!usr.Attempt) 
if(U.cooldown["[Sindex]"]<=0&&Active==1)

as it shouldn't be possible based on how its set. or at least from my understanding of it all
Also, TBH I'd change the way that your cooldowns work.

You don't want to store how long you have left on the cooldown because then you have to loop through the cooldown and count it down for every cooldown, which is a lot more work than is necessary. Instead, store the next time that the cooldown can be used.

U.cooldown["[Sindex]"] = world.time+300 //add 30 seconds to the cooldown


if(U.cooldown["[Sindex]"]<=world.time) //check if the skill is off cooldown yet


When saving, we remove the time that we saved the file from all cooldowns in the list, create a new list of just the remaining ticks, and discard the cooldowns that have expired.

When loading, we loop through the stored cooldowns, and we add the current world.time value back.

It beats the living hell out of maintaining trackers for the timers manually, and will result in way less bugs down the line.

mob
var
tmp/list/cooldown

New()
cooldown = list()

Write(savefile/F)
var/list/l = list(), remaining, time = world.time
..()
for(var/v in cooldown)
remaining = cooldown[v]-time
if(remaining>0)
l[v] = remaining
F["cooldown"] << l

Read(savefile/F)
var/list/l, time = world.time; F["cooldown"] >> l
..()
for(var/v in l)
l[v] += time
cooldown = l
I will definitely rethink the cooldown strategy, super suggestion thanks.

though again I'm still stuck on the proc getting passed Active/Attempt at all on the simultaneous technique
Best response
Frankly, if you don't show us the exact code you are using, we can't help you find the problem.
Let me re explain here.
Ignoring literally everything at all, I've tried stripping it down to just this, any byond macro with the \n new line with same macro on repeat
So example
.click tech1\n .click tech1\n .click tech1
On repeat

If held down can cause the techs to double cast, before it even gets to cooldown, so they will both get through the attempt check and the active check
That simply is not possible. Code cannot run in parallel in DM. Put debugging outpurs in your code. I promise that it is something small.
If the skill is running multiple times and ignoring cooldown because of a macro, then the only way that's happening is if there's a sleep in the proc (or one of its callees) somewhere before the cooldown is checked. Otherwise once the proc begins, it will run to completion.

Checking against the cooldown and whether the player can even use any skill at all should be pretty much the very first thing the proc does.

And that sleep() in the beginning of your Click() is totally bogus, so kill it with fire; at best it's doing nothing and at worst (and most likely) it's contributing to the problem. You should not sleep at all--not in Click() or in the skill itself--until after the cooldown has been checked. And don't rely on waitfor to do the right thing; for robustness' sake you should assume the code will disobey you and assume it will sleep the proc inappropriately no matter what the skill actually does.
I found my way through it, Thank you both for everything. Thank you Ter13 for the cooldown suggestion
What'd it wind up being? Faulty logic or poor timing?
early in the code there were the checks. Between that check and a second check there were some other minor checks I didn't post here, they would mark it as inactive and return which left it wide open for any spam calls to run through before a cooldown had been applied.

so oddly enough the exact bit I pasted on here actually worked as desired, its when I added some other things that it would screw up so you were right

at least that's what I'm guesstimating here, I'm no DM genius by any definition

right now I'm just rethinking my method of getting those other checks through more efficiently/correctly