An open-source Dragon Ball game that I need some minor help in.

Page: 1 2
ID:2086431
 
May 19 2016, 11:37 am
(See the best response by Ter13.)
Code:
var
    RegenerationRate = 100 // default of 10 seconds.


mob
    proc
        Regeneration()
            set background = 1
            spawn while(1)
                while(usr.regenerates)
                    if(!usr.attacked)
                        if(usr.Health < 100)
                            if(usr.Meditate)
                                usr.Health += usr.Regeneration * 0.85
                                if(usr.Health > 100)
                                    usr.Health = 100
                            else
                                usr.Health += usr.Regeneration / 4
                        if(usr.Energy < usr.MaxEnergy)
                            if(usr.Training)
                                return
                            else if(usr.Meditate)
                                usr.Energy += (usr.Recovery * (usr.MaxEnergy * rand(0.005,0.015)))
                                if(usr.Energy > usr.MaxEnergy)
                                    usr.Energy = usr.MaxEnergy
                            else
                                usr.Energy += (usr.Recovery * (usr.MaxEnergy * rand(0.035,0.055)))
                                if(usr.Energy > usr.MaxEnergy)
                                    usr.Energy = usr.MaxEnergy
                        sleep(RegenerationRate)
                    else
                        sleep(100)
        ECheck()
            set background = 1
            spawn while(1)
                while(usr.regenerates)
                    if(usr.Energy < 0)
                        usr.Energy = 0
                    if(usr.Energy > usr.MaxEnergy)
                        usr.Energy = usr.MaxEnergy
                    sleep(20)


Problem description:

I am simply asking for help with this snippet of code, regeneration of energy and health. I am not sure if this has been done in a correct manner and would like a confirmation on whether it is or not.


The full project is found at the following links.


GitHub


Website


BYOND Hub.

May 19 2016, 12:20 pm
No put usr in proc. Ungh.
May 19 2016, 12:22 pm
In response to Lummox JR
If you could, go into a bit further detail :p.
May 19 2016, 12:27 pm
http://www.byond.com/forum/?post=2003116#comment17822051

I've got you covered on the detail, fam.

This will explain what usr and src are and when to use them.
May 19 2016, 12:30 pm
In response to Ter13
Ah, thank you! So I should replace all usr. with src., or remove them entirely (as stated in that post, source is implied thus meaning I don't need src. preceding the variables)?
May 19 2016, 12:37 pm
Yep. That's what the caveman was getting at.
May 19 2016, 1:19 pm
In response to Ter13


Made some slight modifications as well as removing all references to usr in Regeneration, I will be doing the same in other procedures as well. Everything look okay now, if I may?


var
    RegenerationRate = 100 // default of 10 seconds.


mob
    proc
        Regeneration()
            set background = 1
            spawn while(1)
                while(regenerates)
                    if(!attacked)
                        if(Health < 100)
                            if(Meditate)
                                Health += Regeneration * 0.85
                            else
                                Health += Regeneration / 4
                        if(Energy < MaxEnergy)
                            if(Training)
                                return
                            else if(Meditate)
                                Energy += (Recovery * (MaxEnergy * rand(0.035,0.055)))
                                if(Energy > MaxEnergy)
                                    Energy = MaxEnergy
                            else
                                Energy += (Recovery * (MaxEnergy * rand(0.005,0.01)))
                        if(Energy > MaxEnergy)
                            Energy = MaxEnergy
                        if(Health > 100)
                            Health = 100
                        sleep(RegenerationRate)
                    else
                        sleep(100)
        ECheck()
            set background = 1
            spawn while(1)
                while(regenerates)
                    if(Energy < 0)
                        Energy = 0
                    if(Energy > MaxEnergy)
                        Energy = MaxEnergy
                    sleep(20)
May 19 2016, 1:38 pm
I wouldn't set the background setting in those. I read somewhere (believe the source was MrStonedOne) that there is extra overhead that comes with it, and in general I don't think it's necessary there.

Other than that, no real issues I'd say.

Edit: Oh, there are some instances where min()/max() could be used and some conditionals could be avoided.
May 19 2016, 1:44 pm (Edited on May 19 2016, 1:52 pm)
You have loops like this at a few points:
spawn while(1)
  while(condition)
    // code

Unless I'm missing something, this is a very bad idea. As soon as condition is false, the inner loop terminates and the outer loop has no sleep or termination condition, and is a process-hogging loop until condition is true again. This is probably why you have set background = 1, but this is a bad design. You should simply be calling these loops when it makes sense to call them, rather than having them on indefinitely in the background.
May 19 2016, 1:46 pm
In response to Popisfizzy
I didn't even see those; good catch.
May 19 2016, 1:50 pm (Edited on May 19 2016, 1:59 pm)
Best response
 
spawn while(1)
                while(regenerates)
                    if(Energy < 0)
                        Energy = 0
                    if(Energy > MaxEnergy)
                        Energy = MaxEnergy
                    sleep(20)


This is a massive issue. There's no reason to be spawning, and the first while is an infinite loop with no sleeps.

ECheck()
    set waitfor = 0
    while(src)
        while(regenerates)
            //you shouldn't have to do this at all, BTW. You should be gating your changes. If your data is garbage, it's your fault.
            Energy = clamp(Energy,0,MaxEnergy)
            sleep(20)
        sleep(world.tick_lag)


But Echeck() shouldn't even be necessary. Just get rid of it.

mob
    proc
        Regeneration()
            set waitfor = 0
            while(src)
                while(regenerates)
                    if(last_attacked-world.time>100)
                        if(regen_health&&Health<MaxHealth)
                            Health = clamp(Health + Regeneration * regen_health,0,MaxHealth)
                        if(regen_energy&&Energy<MaxEnergy)
                            Energy = clamp(Health + Recovery * regen_energy,0,MaxEnergy)
                        sleep(RegenerationRate)
                    else
                        sleep(world.tick_lag)
                sleep(world.tick_lag)


Instead of doing all the checks like "is_training" or "is_meditating", just change the regen_health/regen_energy scalars based on the user's state.

So when they enter training:

regen_energy = 0

When they enter meditation:

regen_health = 0.85

When they exit meditation:

regen_health = 0.25

Instead of checking if the player has been attacked and unsetting that later, just set the last_attacked variable to the time of the most recent attack whenever the player is hit.

last_attacked = world.time

A lot more flexible in structure than a boolean.

Also, make sure you define clamp() it's super useful:

#define clamp(v,l,h) min(max(v,l),h)
May 19 2016, 5:12 pm
In response to Ter13


Thanks a lot for this help, I will be doing this immediately! I'm not an expert with BYOND scripting :p.



If I may ask, why should I clamp? Curious.

May 19 2016, 5:37 pm
Ter, what is that waitfor setting? I couldn't find anything about it at "f1"
May 19 2016, 6:16 pm
Ter, what is that waitfor setting?

Waitfor is old. It was replaced by spawn(), but spawn() was not a replacement for waitfor.

Basically, it means that the calling function should not wait around on the proc if waitfor is set to 0.

If I do something like this:

mob/proc/regen()
    while(src)
        hp += 1
        sleep(world.tick_lag)

mob/Login()
    regen()
    ..()


The login will never complete because it's waiting on regen().

Most people fix this with a spawn:

mob/proc/regen()
    while(src)
        hp += 1
        sleep(world.tick_lag)

mob/Login()
    spawn() regen()
    ..()


But this is bad because it involves the scheduler in business it doesn't need to be involved in. Plus, it's annoying to always be spawn()ing your code.

Let's say you've got two of these things:

<dm>
mob/proc/regenhp()
    while(src)
        hp += 1
        sleep(world.tick_lag)
mob/proc/regenmp()
    while(src)
        mp += 1
        sleep(world.tick_lag)


mob/Login()
    spawn()
        regenhp()
        regenmp()
    ..()


Now the problem is that regenmp() never gets called because it's waiting on regenhp() to finish --and it never will.

So we have to put a second spawn:

mob/Login()
    spawn() regenhp()
    spawn() regenmp()
    ..()


Are you starting to see how ugly this is? Add to that if you bring on someone with a poor understanding of blocking functions to program for you and god forbid they forget the spawn, or start spawning every function willy-nilly because they are under the impression that spawn make game faster. (Like so many programmers are around here)

Sure, we could just fix it like this:

mob/proc/regenhp()
    spawn()
        while(src)
            hp += 1
            sleep(world.tick_lag)

mob/proc/regenmp()
    spawn()
        while(src)
            mp += 1
            sleep(world.tick_lag)
mob/Login()
    regenhp()
    regenmp()
    ..()


Sure, the Login function is a little cleaner, but now regenhp() and regenmp() are uglier.

Let's just swap to a waitfor:

mob/proc/regenhp()
    set waitfor = 0
    while(src)
        hp += 1
        sleep(world.tick_lag)

mob/proc/regenmp()
    set waitfor = 0
    while(src)
        mp += 1
        sleep(world.tick_lag)

mob/Login()
    regenhp()
    regenmp()
    ..()


BYOND makes heavy use of waitfor settings under the hood. Ever wonder why you can't sleep in Move(), Enter(), Exit(), Cross() or Uncross()? Because sleeping makes waitfor procs instantly return whatever value is set to ".". That means that any sleep() in these procs will break anything below them for actually determining the return value of the proc itself. This is why I argue that you should never sleep OR spawn in either of these procs.

The waitfor setting is not a total replacement for spawn(). There are places where you will want to use waitfor and places you want to use spawn. The distinction is that you control the ejection point of spawn(), while waitfor ejects only after the first sleep(). So if you have heavy code in your function that's set to waitfor above the first sleep call, you may be better suited with a spawn(). Most functions should not be very heavy at all though. There are very few algorithms where this should ever be true.

Basically, I only use spawn() for state delays and controlling code flow. If you find yourself using it all over the place, I promise you that are you are using it incorrectly.
May 19 2016, 6:21 pm
In response to E52680
E52680 wrote:


Thanks a lot for this help, I will be doing this immediately! I'm not an expert with BYOND scripting :p.



If I may ask, why should I clamp? Curious.


clamp() is a useful pattern that you are using everywhere in your program.

You see all the places where you are checking:

if(value<0)
    value = 0
else if(value>max_value)
    value = max_value


That's clamping logic. clamp() ensures that the value v is set to a value between l (low) and h (high). That's the same thing that you are doing right now with those if statements, but with less redundant logic and fewer lines of code.

Once you understand that those four lines of code you were using are called a "clamp", and you understand that you are going to be using those EVERYWHERE in your code, having a simple little instruction to do it all on one line is a great shortcut for your fingers as well as being computationally faster:

clamp(value,0,max_value)

VERSUS:

if(value<0)
    value = 0
else if(value>max_value)
    value = max_value


Clamping patterns are everywhere in programming. clamp() is a standard function that almost every programming language offers as part of its built-in toolset. Unfortunately, BYOND isn't one of them, hence the preprocessor definition I left you in the above post.
May 19 2016, 6:25 pm
In response to Ter13
Sadly, I still don't fully understand what you are saying.

Sorry if I'm a bit annoying, but could you dumb it down a bit more, if possible? I probably know exactly what you are trying to explain to me, just not in the same terms you have used it in.
May 19 2016, 6:27 pm
I have a question about sleep(), why are you using world.tick_lag instead of sleep(10) for example?
May 19 2016, 6:36 pm
clamp: Ensure that a value is between a low and high limit.


if(value<0)
    value = 0
else if(value>max_value)
    value = max_value


^That is a logic pattern. Repetitive pieces of code that do a specific task are called patterns.

In programming, there are many ways to achieve the same result. Not all ways are equal. Some are faster. Some require more lines. Some are more readable. There are many ways that you can judge one pattern being better than another.

#define clamp(v,l,h) min(max(v,l),h)


That's a preprocessor macro. It's sort of like a fake proc. The clamp proc doesn't actually exist. Instead, when you compile your game, any use of clamp() is replaced with min(max(v,l),h).

min() returns the lowest of two or more values.
max() returns the highest of two or more values.

if(value<0)
    value = 0
else if(value>max_value)
    value = max_value

//does the same thing as:

value = min(max(value,0),max_value)


The difference between the two is the bottom one is less lines of code, which is good for readability. The problem is that it's a mess, which is bad for readability. Readability means how well the code translates into human-readable ideas.

That's why we're using a preprocessor macro:

#define clamp(v,l,h) min(max(v,l)


How the code looks to you:

value = clamp(value,0,max_value)


How the code looks to the compiler:

value = min(max(value,0),max_value)



TL;DR: Defining the clamp macro and using it is a convenient way to save you a lot of typing. That's all I was trying to explain.

Also, look up the phrase GIGO. That's why you should clamp.
May 19 2016, 6:39 pm
In response to Ter13
Ter for president!!
:O thx dude, you are awesome <3
May 19 2016, 8:27 pm
Clamping is a form of what's called "validation", meaning you want to make sure the inputs you get are valid.

For instance, if you write a verb into your game that lets you give gold to another users, you want to make sure the give amount is no less than 0--or else you can steal from another player--and that it's no more than you have. You also probably want to ensure it's an integer value, so you can't transfer 0.5, or worse 0.1 which is subject to rounding error. Any time you get data from a user, it should be validated.
Page: 1 2