[vJASS] GetTerrainZ

[SA Dashie]

library GetTerrainZ /* v1.1.0.0, created by DeathChef

    Description
        
        The most effective and efficient way to get the z coodrinate
        of the terrain through the x and y coordinates
        
        What is useful about this snippet?
            
            - It makes using a local real redundant in the right situation and helps
              keep things clean
            
            - If you need to get the TerrainZ two or three times in a single instance
              you only need to do two commands with the second and third one being even
              more simple
            
            - Can be blended in with ITE(if then else) in situations when using Terrain Z
              could be used either once or three times
        
    Fields
        
        constant location locZ
        
        real storedZ
        
    Functions
        
        function GetTerrainZ takes real x, real y returns real
            - Plain returning z coordinate without add-ons
        
         function StoreGetTerrainZ takes real x, real y returns real
            - Does same as run except value is stored into global
        
        //Single line functions
        
        function GetStoredZ takes nothing returns real
            - Returns stored value
            USE THIS LINE INSTEAD
                storedZ
        
        function GetLastZ takes nothing returns real
            - Returns z coordinate from last instance
            USE THIS LINE INSTEAD
                GetLocationZ(locZ)
    
    
*/


    globals
        constant location locZ = Location(0, 0)
        real storedZ
    endglobals
    
    function GetTerrainZ takes real x, real y returns real
        call MoveLocation(locZ, x, y)
        return GetLocationZ(locZ)
    endfunction
    
    function StoreGetTerrainZ takes real x, real y returns real
        call MoveLocation(locZ, x, y)
        set storedZ = GetLocationZ(locZ)
        return storedZ
    endfunction
    
    function GetStoredZ takes nothing returns real
        return storedZ
    endfunction
    
    function GetLastZ takes nothing returns real
        return GetLocationZ(locZ)
    endfunction
    
endlibrary

Example:
The example doesn't compile or have a real use but you should get the point.

function Example takes integer UnitIndex returns real
    
    local unit u = TestU[UnitIndex]
    local real PredictedUnitZ = UnitZ[UnitIndex] - momentum
    
    if StoreGetTerrainZ(GetUnitX(u), GetUnitY) >= PredictedUnitZ then
        //If this condition is a lot less likely to pass you
        //could just use "GetTerrainZ" instead and
        //
        //return GetLocationZ(locZ)
        
        return storedZ
    else
        return PredictedUnitZ
    endif
    
endfunction

The one [here] I find incomplete to how I like doing things.

 

[BPower]

location l could use a better name and be accessible
from other libaries, thus readonly.

Jass snippets should follow the JPAG code convention.

Why not store a returned z value in a local real?
StoreRun and GetStoredZ isn't reliable, as a new thread could
run Run and overwrite that value.
In general StoredZ should be readonly or private.

Function names could be more generic i.e struct name LocZ
--> method name get(x, y) or move(x, y).

ReRun turns out to be useless, as you could
use a local and to store returned z value.
I understand that your code would inline,
but you could also use GetLocationZ(LocZ.loc) directly.
in case location loc would be readonly.

Using a struct doesn't seem useful to me ( except for personal read-ablity issues ).
A simple function would generate less code and look cleaner to me.
From a view of backwards- compatibilty, the function name should be GetTerrainZ.

Can you give an example why this snippet
is significant better than the original?

For me you are trying to achieve micro optimization, but 0 actual
useful extra functionality.

 

[SA Dashie]

Ok thanks for the reply.
That's a lot of useful information and a good argument to challenge.

I'll reply back soonish as I don't have access to JNGP atm.

 

[SA Dashie]

Updated 1.0.0.0 -> 1.1.0.0
- No longer coded with usage of a struct
- Renamed functions and variables
- Added a description for what is useful in this snippit
- Made an example for usage


If no one sees a possible use for this now I'll probably just move on.

 

[looking_for_help]

The one [here] I find incomplete to how I like doing things.

Why exactly?

IMO the library you linked already provides everything a user needs. Yours only adds a global to cache its result. However, if GetTerrainZ is not particular expensive (which I doubt) then I don't really see the advantage of using this. Even worse, I see some significant disadvantages.

Why not just let the user cache the result in a local (or even an own global) himself? Thats much more flexible and "better" anyway. Globals should be avoided if possible. This system also allows only one cached value at a time.

So what happens if you have two systems that both rely on this resource and, say both read the global in a timer callback. Now system 2 changes the stored location, how will system 1 get notified that "his" location is invalidated? It will continue to read the cached value, which now belongs to system 2, and break.


Not only that the functionality can be easily implemented by the user if needed, it also should be implemented by the user. Because otherwise you create unnecessary dependencies that can potentially break other systems like explained above.

 

[SA Dashie]

IMO the library you linked already provides everything a user needs.

This already uses everything in that library but just replaces it and adds additional functionality so I don't see any issue here.

Why not just let the user cache the result in a local (or even an own global) himself? Thats much more flexible and "better" anyway. Globals should be avoided if possible. This system also allows only one cached value at a time.

This lets you set and forget to let you keep your code minimal without a use of a local and I don't see why this would make your code less efficient so I don't see an issue here either.

So what happens if you have two systems that both rely on this resource and, say both read the global in a timer callback. Now system 2 changes the stored location, how will system 1 get notified that "his" location is invalidated? It will continue to read the cached value, which now belongs to system 2, and break.

There shouldn't be an issue here if your aware that you should keep this safe like how you shouldn't use terrain deformation without revealing the whole map or at least where the terrain has been deformed(?).

Just don't use it in unsafe situations and use it for it;s intentions and everything should function without issue.

Like you said with using locals to store the gained Z value you could always still use that without this system having a disadvantage compared to the linked one considering that this is basically an extension.

Not only that the functionality can be easily implemented by the user if needed, it also should be implemented by the user. Because otherwise you create unnecessary dependencies that can potentially break other systems like explained above.

That's the thing though, if you need this system and the other system you could just remove the other system and nothing would break (although keeping the other system would make the code break).

The users can do whatever the want with this snippet I don't really mind I've had my fun haha.

Remember that this is more of an extension than anything else.
If you think that there is a way to add more cached values to the system at once then I'm not stopping you from givving suggestions.

 

[looking_for_help]

This already uses everything in that library but just replaces it and adds additional functionality so I don't see any issue here.

Well I do. Because I can easily rewrite your system like this:

library GetTerrainZCached uses GetTerrainZ
    globals
        real storedTerrainZ
    endglobals
endlibrary

And thats it, no need for extra functions:

set storedTerranZ = GetTerrainZ() // Store it
call BJDebugMsg(R2S(storedTerrainZ)) // Use it

Why should I mind learning an extra API just for storing and reading a single value?

This is just providing a getter and setter for a global variable, thats just not enough for a Jass system, not even for a snippet. Its way to simple and adds zero functionality.

This lets you set and forget to let you keep your code minimal without a use of a local and I don't see why this would make your code less efficient so I don't see an issue here either.

In any realistic scenario you will have to use a local/your own global anyways to avoid dependencies introduced by mutable global state.

There shouldn't be an issue here if your aware that you should keep this safe like how you shouldn't use terrain deformation without revealing the whole map or at least where the terrain has been deformed(?).

Just don't use it in unsafe situations and use it for it;s intentions and everything should function without issue.

Thats not an argument for, but against this system.

If someone wants to create a system and use this library, he can't know if at some day any other system will interfere with his code. So he will decide against this and just use his own private global to store the result if required. That way the code is as clean as with yours, just one global added (even better, he doesn't need an aditional dependency to his library).

Like you said with using locals to store the gained Z value you could always still use that without this system having a disadvantage compared to the linked one considering that this is basically an extension.

If I use a system I expect it to solve a problem for me. This has just the same problems like a raw global variable.

That's the thing though, if you need this system and the other system you could just remove the other system and nothing would break (although keeping the other system would make the code break).

Forcing a user to remove a system just for a micro optimization (which you didn't even prove as neccessary/efficient yet) is a no-go. A system should make life easier, not harder.

 

[BPower]

I vote for gy.

For me the snippet equals the original one. It's just less safe to use.
There is no way to determine if storedZ is valid or already overwritten.

Allocating a local variable, which is btw 100% reliable is everything, but not expensive.

 

[SA Dashie]

Ok thanks, I vote for graveyard too.

I see it's far to tacky for something that doesn't give the users anything really that worth it for the effort needed.

I see it serves no extra real functionality for users

This information should be usefull for keeping in the back of my head for future coding.

Btw

Well I do. Because I can easily rewrite your system like this:

library GetTerrainZCached uses GetTerrainZ
    globals
        real storedTerrainZ
    endglobals
endlibrary

And thats it, no need for extra functions:

set storedTerranZ = GetTerrainZ() // Store it
call BJDebugMsg(R2S(storedTerrainZ)) // Use it

Why should I mind learning an extra API just for storing and reading a single value?

This is just providing a getter and setter for a global variable, thats just not enough for a Jass system, not even for a snippet. Its way to simple and adds zero functionality.

Lol I think I just realized that doing this as a local in your own code doesn't really provide any less action calls over all and using my function call StoreGetTerrainZ with the variable only has less script in the users code itself.

So basically if Garfield the creator of the linked GetTerrainZ made the location variable accessible this system wouldn't have any more efficiency at all.

 

[looking_for_help]

Gy'd on request.