• Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
  • Read Evilhog's interview with Gregory Alper, the original composer of the music for WarCraft: Orcs & Humans 🔗Click here to read the full interview.

[System] HeroStat

It would be nice if you added comments to your more obscure code sections.

This, for example, means nothing to me:
JASS:
            if (oin[this]) then
                return in2[this]
            endif

If you're going to use unreadable variable names, you should at least consider marking your code well so that people like me can decide if something could be improved with your code.

There's also the fact that, when compiled with the verbose HeroStat__ prefix, your short variable names don't accomplish much.
 

LeP

LeP

Level 13
Joined
Feb 13, 2008
Messages
545
Wow, even considering code like this to get approved really shows what level this database has reached.
I have no problem if you write your code shitty like that, but as soon as you publish it we also have to deal with it.
  1. Someone has to read it to approve it. Have fun, code-database moderator!
  2. I use this in my map (god behold!) and i want a change. I have to work through that code base. I'd rather write it myself than work with that code.
  3. Blizzard fucks something up again, and we have change the code to reflect these changes (like handlevars->table).
Even as the author of the code it gets hard after you didn't touched your code for some time.

And most of the time code is much more read that it is written. Write it good once, never have problems again.
 
oin is override int

o refers to override and in refers to int

I can read these variable names perfectly. I mostly use acronyms for the short names =P.

Plus at the top, most every variable has a description. Only ones without descriptions are the 3 obvious ones ;D. And, again I use acronyms, so they are fairly obvious through the code when you read the descriptions through one time ;P.

I can read the code just fine atm and I haven't looked at this in about a month ; ).

I use this in my map (god behold!) and i want a change. I have to work through that code base. I'd rather write it myself than work with that code.

lol, the code is extremely simple =). Also, I use it in all of my maps with heroes : p.


Actually, I think what's smarter is to use this to initially balance the map and log how it upgrades the hero str, agility, and intelligence and what it uses for starting values. It's a bit more work, but it makes the map a bit more optimal as the library can be deleted out of the map for the final version =).
 

BBQ

BBQ

Level 4
Joined
Jun 7, 2011
Messages
97
I can read these variable names perfectly. I mostly use acronyms for the short names =P.]

You don't get it. You can fix the optimizer to work with those AI natives and TriggerRegisterVariableEvent() by manually editing the optimized war3map.j. Doing so takes less than 2 minutes.

If you care so much about variable length, then you should be actually advocating the optimizer's usage, because even if you have private real array gr, it'd end up being complied to real array HeroStat__gr unless the map is optimized afterwards. And that's quite a long name, isn't it?
 
Yes it is ;\.

The optimizer just needs to be fixed and I don't think the source code is available, so that means recoding it from scratch, which nobody wants to do, lol.


Even if the source code was available, I doubt it'd be any good. Look at the jasshelper source code. That code is a nightmare.



Oh yes, and why has nobody complained about the BigInt code? I even struggle reading the BigInt code, lol...


I doubt that any of it was read or looked over when it was approved, lol.


You say this code is bad, but this is infinitely worse : p
JASS:
            loop
                set y=n[y]
                exitwhen e[y]
                set q=y
                loop
                    exitwhen z>v[q]
                    if (e[n[q]]) then
                        if (0==n[0]) then
                            set i=i+1
                            set a=i
                        else
                            set a=n[0]
                            set n[0]=n[a]
                        endif
                        set p[a]=q
                        set n[a]=td
                        set n[q]=a
                        set p[td]=a
                        set v[a]=0
                    endif
                    set v[n[q]]=v[n[q]]+v[q]/z
                    set v[q]=v[q]-v[q]/z*z
                endloop
            endloop

And I can actually read most of that
 
Level 10
Joined
Nov 28, 2008
Messages
655
"Here is this thing I made for you guys to look at!"
"We can't read it"
"But I can so... ?"


That is what you really just did. Take a step back, and you will see what I mean.


Programmers are jerks. Get over it.

You don't do things our way, then don't expect any kindness.

Variable names should be long enough to know what they mean, no matter who reads it. Just because you understand them right after writing them, does not mean that they are 'acceptable'.


In a 'real-life' code review? This would get eaten alive.

It is also more acceptable to comment throughout the code, instead of making one giant comment block at the top and then not doing any inside. Just a heads up.

I don't want to have to scroll up to see what things are doing and what the variable 'og[this]' is referring too.



"You say this code is bad, but this is infinitely worse : p"
Please don't say that ever again...


I will repeat, programmers are jerks. Do not step up and try to make things to be looked at by them and hand them this crap. You will be sorely rejected and saddened.

There are reasons for being this strict and stringent when it comes to programming, and it is appealing to a very specific type of persona.

-----------------


I like the idea, it is pretty nifty.
 
I don't code this way in a professional environment, I only code this way for wc3 =).


I also don't expect users of the resource to modify the resource or to read the actual code in the resource =). I know moderators have to read the code, but I don't expect users to.


The coding I do for wc3 is nightmarish and would never be accepted in a professional environment. The only way I'd start coding normally for warcraft 3 is if there was an inline keyword and a fully working optimizer.


This, for example, is an allocation operation, one that I would normally create a method for-
JASS:
if (0==n[0]) then
    set i=i+1
    set a=i
else
    set a=n[0]
    set n[0]=n[a]
endif


We've been over how I code for wc3 countless times and everyone already knows that I'm not going to stop doing things this way until the tools get better ; P. There's no use in arguing over the readability of the code. The best I'm going to do are comments like those found in Encoder =), which are the same style of comments that I use professionally.


If you guys want professional level code with professional level comments, then someone work on a working optimizer and an inline keyword for vjass. If that's done, then I will happily update all of my resources.


Another fantastic thing would be an Lua block for Lua code so that the //! i characters aren't needed.


This script in particular is extremely simple.

This is pretty much the entire library. The rest are just the stored values.
JASS:
    private module Init
        private static method lvl takes nothing returns boolean
            local unit u = GetIndexedUnit()
            local HeroStat i = GetUnitId(u)
            local integer l = GetHeroLevel(u)
            local real p
            local real p2
            if (IsUnitType(u, UNIT_TYPE_HERO)) then
                set p = i.base*i.strength/STR_COST
                set p2 = i.tgrow*i.strength/STR_COST
                if (l > 1) then
                    set str[i] = GetHeroStr(u, false)-R2I(p+p2*(l-2))
                else
                    set str[i] = GetHeroStr(u, false)
                endif
                call SetHeroStr(u, R2I(p+p2*(l-1)+str[i]), true)
                
                set p = i.base*i.agility/AGI_COST
                set p2 = i.tgrow*i.agility/AGI_COST
                if (l > 1) then
                    set agr[i] = GetHeroAgi(u, false)-R2I(p+p2*(l-1))
                else
                    set agr[i] = GetHeroAgi(u, false)
                endif
                call SetHeroAgi(u, R2I(p+p2*(l-1)+agr[i]), true)
                
                set p = i.base*i.intelligence/INT_COST
                set p2 = i.tgrow*i.intelligence/INT_COST
                if (l > 1) then
                    set inr[i] = GetHeroInt(u, false)-R2I(p+p2*(l-1))
                else
                    set inr[i] = GetHeroInt(u, false)
                endif
                call SetHeroInt(u, R2I(p+p2*(l-1)+inr[i]), true)
            endif
            set u = null
            return false
        endmethod


Again, as you see, it's incredibly simple.


For example
i.base*i.strength/STR_COST


Base would be the amount of points there are total, strength would be the percent of points going to strength, and STR_COST would be how many points each strength point costs.


i.tgrow*i.strength/STR_COST


tgrow refers to the amount of points given from leveling. Base is amount of points given at level 1.


As can be seen, the setting isn't cumulative. This means that one can set a hero to level 10 instantly and have the right strength points, or set them to level 10 and then level 5.
call SetHeroStr(u, R2I(p+p2*(l-1)+str[i]), true)


That equation will work for any level as it accounts for both level 1 and extra levels. The l refers to level, and because 1 is being subtracted, if a unit is level 1, p2 will be 0. str would be the strength a unit has that was not given via this script.


That is essentially the entire library. As I said, it's very simple.
 
most of the variable names were totally random

That's like.. Vexorian evil O.O

edit
I usually use:
- u,v,w -> units
- x,y,z -> coordinates
- a -> angle
- r -> a real
- s -> a string
- i,j,k -> indices in multidimensional loops :p
- m,o,q -> Players
- b,e -> a boolean
- t -> trigger
- n,p -> next and previous in linked list
etc..
 
Level 10
Joined
Nov 28, 2008
Messages
655
Why does the quality of the tools have anything to do with using uselessly short variable names? I don't understand that.

I can do my pointer yoga in C in notepad, and I won't need to make every variable names single or double letter, lol

I don't understand, :ogre_haosis:
 
Why does the quality of the tools have anything to do with using uselessly short variable names? I don't understand that.

I can do my pointer yoga in C in notepad, and I won't need to make every variable names single or double letter, lol

I don't understand, :ogre_haosis:

because they aren't uselessly short. Longer names in JASS make the code run slower =P.
 
Top