GetUnitArmor

Level 13
Joined
Dec 12, 2012
Messages
1,000
the purpose of the adapter, is to give a common interface to the client. its fine if it gets modified by the user as long as the api remains the same.

Its not fine to modify library code because then you could also just let the user directly modify the GetUnitArmor library code. A user should never have to modify a library, but just cofigure and use it.

having to use <x> exists directly on the resource is kinda shitty from my point of view.

Again, why?

"Because I think its shitty" is not an argument. Its an implementation detail anyway and its a perfectly clean solution because it allows users to implement their code seperatly from the library which uses that code.
 
Level 9
Joined
Sep 19, 2011
Messages
527
Its not fine to modify library code because then you could also just let the user directly modify the GetUnitArmor library code. A user should never have to modify a library, but just cofigure and use it.

it doesn't matter the content of the wrapper, the only what matter is the api (enable, disable, etc.). anyone is able to modify the content of these functions, the idea is to provide an interface so resources like this one can depend on. my idea was to fill this wrapper with the approved dds found here so users wouldn't have to do it themselves.

Again, why?

i found way more productive having to call a simple function than having to create a module, make it implement another module, check if a method exists and then call it.
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
the idea is to provide an interface so resources like this one can depend on. my idea was to fill this wrapper with the approved dds found here so users wouldn't have to do it themselves.

I know and there is nothing wrong with providing a set of such DDS for library makers. But its not good if every library that uses that wrapper requires a different version of it.

Imagine the following:

JASS:
// DDSWrapper here supports DDS1, DDS2 and DDS3
library GetUnitArmor uses DDSWrapper

// DDSWrapper here supports DDS1, DDS2, DDS4 and DDS5
library Shield uses DDSWrapper

// DDSWrapper here supports DDS1, DDS2 and DDS6
library ChargeableAttack uses DDSWrapper

So that might happen if everyone has to tinker around for himself in such a wrapper and in each map they grow into different directions. At the end you will have different versions of the wrapper and the user will have to merge them manually if he wants to use all three libraries above with, say, DDS5.


i found way more productive having to call a simple function than having to create a module, make it implement another module, check if a method exists and then call it.

I don't get what you mean with productive here. All the user has to do is to write:

JASS:
module DDSDisableModule
    static method DisableDDS takes nothing returns nothing
        call SpecificDDSDisableMethod()
    endmethod
    static method EnableDDS takes nothing returns nothing
        call SpecificDDSEnableMethod()
    endmethod
endmodule

Thats almost as short as it gets? What I think is not productive is having a huge wrapper like this:

JASS:
library DDSWrapper
    public function Enable takes nothing returns nothing
        static if (DDS_1 exists) then
            call <dds1 function to enable>
        endif
        static if (DDS_2 exists) then
            call <dds2 function to enable>
        endif
        static if (DDS_3 exists) then
            call <dds3 function to enable>
        endif
        static if (DDS_4 exists) then
            call <dds4 function to enable>
        endif
        static if (DDS_5 exists) then
            call <dds5 function to enable>
        endif
        // Add your static if for enable here!
    endfunction
    public function Disable takes nothing returns nothing
        static if (DDS_1 exists) then
            call <dds1 function to disable>
        endif
        static if (DDS_2 exists) then
            call <dds2 function to disable>
        endif
        static if (DDS_3 exists) then
            call <dds3 function to disable>
        endif
        static if (DDS_4 exists) then
            call <dds4 function to disable>
        endif
        static if (DDS_5 exists) then
            call <dds5 function to disable>
        endif
        // Add your static if for disable here!
    endfunction
endlibrary

And then expect the user to find the correct locations were to add his static ifs himself, add the existing conditions into the static ifs himself and add the enabling and disabling code himself.

As a user I would definitly prefer the first option since I don't have to dig through that wall of static ifs and add them myself. Instead my enabling/disabling code is within a module in its own trigger. Clean and readable.
 
Level 9
Joined
Sep 19, 2011
Messages
527
its gonna be the same thing, but instead of using a module you would be using functions (meaning no need to implement support for all dds, just the one the user is currently using).
do keep in mind that your approach requires the developer to create a module optionally implementing this special (?) module and check for available methods to exist.
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
I think you are misunderstanding how this is meant to be used. I will explain in more detail.

its gonna be the same thing, but instead of using a module you would be using functions (meaning no need to implement support for all dds, just the one the user is currently using).

Wait, you are mixing two things now. We should first look at the audience of the DDS selection mechanism.

1. "Normal" users that just use a DDS in their map and need to deactivate it, but don't create a public library which requires turning off a DDS. Such users don't really have a useage for a wrapper at all, because they use a specific DDS in their map and their map-specific code doesn't have to be reusable at all.

2. Users that create a public library that requires to turn off (any) running DDS. Only those users really need such a Wrapper to be shipped with their library to make the library independent of the DDS. The Wrapper shipped with these libraries must have support for all/most DDS because otherwise the library isn't independent of the DDS.

So, now if a user 1 wants to use two different libraries created by two different users 2. Then its a very bad thing if those two libraries have different versions of the Wrapper. The Wrapper should be an own library which noone except the author has to modify.


do keep in mind that your approach requires the developer to create a module optionally implementing this special (?) module and check for available methods to exist.

No it does not :D
Maybe the example wasn't detailed enough, say you create your wrapper like this:

JASS:
library DDSWrapper
    private struct DDSDisabler extends array
        implement optional DDSDisableModule
    endstruct

    public function Enable takes nothing returns nothing
        // List with all predefined DDS
        static if (DDS_1 exists) then
            call <dds1 function to enable>
        endif

        // Mechanism to handle unknown DDS
        static if DDSDisabler.DisableDDS.exists then
            call DDSDisabler.DisableDDS()
        endif
    endfunction
    // ...
endlibrary

So neither user 1 nor user 2 will have to use any static ifs and check for method existance. Only the one writting the wrapper will have to do this once.

Or, if someone doesn't wan't to use such a wrapper he will have to do it in his library himself of course.
 
Level 14
Joined
Jun 27, 2008
Messages
1,325
Tbh i would suggest to stick to the very basic functionality of this system and ignore advanced stuff like damage detection.

I think these kinds of systems are mostly used for "simpler" maps without complex triggersystems, so it would be nice to have a small code without many interactions with other systems.

For more complicated maps where you manipulate unit stats a lot, you usually can get a units stats directly (faster, higher accuracy, more reliable)
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
You might be right that this will mostly used by "simpler" maps and should therefore not be overcomplicated. However, even in simple maps its not uncommon to use some DDS. If someone uses an Armor-Detection system, why not a Damage Detection system too?

Because on the other hand this will just break any existing DDS if it doesn't account for them, which I don't think is acceptable even for "simple" resources. Because one of the most "natural" usage of this system is exactly the problematic one: Modify damage based on current Armor of the target. There you need both, a DDS and an Armor Detection system.

Also I don't think this will complicate the system a lot... its basically just a bunch of static ifs, which are implementation details and never expected to be seen by the end-user of this system at all. Unfortunatly there isn't really a perfect solution to this problem, only a new compiler feature could probably achive this...
 
Level 13
Joined
Dec 12, 2012
Messages
1,000

Well, I'm not 100% sure about that because I never thought that through completly...

But a compiler could register all calls to call TriggerRegisterUnitEvent(t, u, EVENT_UNIT_DAMAGED) and then generate a function GlobalDisableDamageEvent which disables all existing trigger like that.

So if a user writes

JASS:
function foo takes nothing returns nothing
    call TriggerRegisterUnitEvent(CreateTrigger(), someUnit, EVENT_UNIT_DAMAGED)
endfunction

the compiler could generate something like:

JASS:
function foo takes nothing returns nothing
    local trigger trg = CreateTrigger()
    // save trg in some data structure
    call TriggerRegisterUnitEvent(trg, someUnit, EVENT_UNIT_DAMAGED)
endfunction

function GlobalDisableDamageEvent takes nothing returns nothing
    // load all triggers from the datastructure and disable them with call DisableTrigger(trg)
endfunction

So basically an automatic hooking of TriggerRegisterUnitEvent(t, u, EVENT_UNIT_DAMAGED) (or potentially any other event if that makes sense).
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
By the way lfh zinc does it for

Actually we would need runtime checks anyway, so with correctly implemented hooks it should work.


JASS:
function TriggerRegisterUnitEventEx takes trigger whichTrigger, unit whichUnit, unitevent whichEvent returns event
	if whichEvent == EVENT_UNIT_DAMAGED then
		// Save whichTrigger handle in a data structure
	endif
	return TriggerRegisterUnitEvent(whichTrigger, whichUnit, whichEvent)
endfunction

// ...
hook TriggerRegisterUnitEventEx before TriggerRegisterUnitEvent // Example syntax

That should be enough to do the trick...


However, we are getting off-topic, so lets focus on this resource ;)
 
Level 16
Joined
Jul 17, 2011
Messages
1,856
um listen there is no need to make this compatible with dds's and shield systems and what not this is a system for small maps, if youre doing a big project then you should go with a custom combat system ( where armor, damage ,armor type ,damage type, shields ,healing etc are handled by the system and not the game engine )

this just adds a little extra SIMPLE functionality
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
um listen there is no need to make this compatible with dds's and shield systems and what not this is a system for small maps, if youre doing a big project then you should go with a custom combat system ( where armor, damage ,armor type ,damage type, shields ,healing etc are handled by the system and not the game engine )

I think you greatly underestimate how much work it is to code a whole combat system with everything from scratch.

this just adds a little extra SIMPLE functionality

Again, there it no additional complexity for the user of the system. The user doesn't have to care about anything, it will just work.

I know quite a lot of huge maps that don't use from scratch damage engine.

+1
 

Dr Super Good

Spell Reviewer
Level 57
Joined
Jan 18, 2005
Messages
26,447
LnAprox needs its own thread to avoid risk of crashing. I recommend setting its parameter to a global, then call either ExecuteFunc(SCOPE_PRIVATE+"LnAprox") or ForForce(bj_FORCE_PLAYER[0], function LnAprox)
Or just a better implementation of Ln is needed with more predictable/reliable execution complexity. I hacked that one together just to show it is possible.
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
Or just use lfh Math's library.

That would be better anyway, both in terms of accuracy and performance.

Accuracy comparison (9 digits total accuracy, wrong digits marked red):

Code:
x       ln(x)         Math.ln(x)       LnAprox(x)
--------------------------------------------------
0.2    -1.60943791   -1.60943[COLOR="Red"]814[/COLOR]      -1.60[COLOR="Red"]811994[/COLOR]
2       0.69314718    0.69314[COLOR="Red"]842[/COLOR]       0.69[COLOR="Red"]277619[/COLOR]
5       1.60943791    1.60943[COLOR="Red"]802[/COLOR]       1.60[COLOR="Red"]803219[/COLOR]
50      3.91202301    3.912023[COLOR="Red"]49[/COLOR]       [COLOR="Red"]25725.64...[/COLOR]
1000    6.90775528    6.907755[COLOR="Red"]33[/COLOR]       [COLOR="Red"]25729.46...[/COLOR]
10000   9.21034037    9.21034[COLOR="Red"]144[/COLOR]       [COLOR="Red"]25729.64...[/COLOR]

LnAprox is very inaccurate even for small numbers and only produces valid values in very small interval. Math.ln is highly optimized for vJass real precision type as demonstrated in the table above and works even for very large numbers.

Performance comparison (Release mode, OP limit: higher is better, worse result marked red):

Code:
x       Math.ln(x)    LnAprox(x)
--------------------------------
0.2     1679          [COLOR="Red"]1330[/COLOR]   
2       1986          [COLOR="Red"]1324[/COLOR]
5       1760          [COLOR="Red"]1324[/COLOR]
50      1312           [COLOR="Red"]495[/COLOR]
1000    1045           [COLOR="Red"]495[/COLOR]
10000   1045           [COLOR="Red"]495[/COLOR]

LnAprox performance is stable around ~1320 OPs (considering only valid results, because it produces wrong values for x > 50) in this test. Math.ln is considereably faster in all benchmarks and should therefore be prefered for both performance and accuracy reasons.


You can also just directly use the ln function from the Math library, no need to add a massive dependency for just one function here.
 

Dr Super Good

Spell Reviewer
Level 57
Joined
Jan 18, 2005
Messages
26,447
LnAprox is very inaccurate even for small numbers and only produces valid values in very small interval. Math.ln is highly optimized for vJass real precision type as demonstrated in the table above and works even for very large numbers.
Too bad negative armor is capped to a very small range, hence why LnAprox works. No need for it to support very large numbers since there are no very large numbers in this application?

The range of Ln used is...
Ln(X) -> 0<X<1

Additional accuracy could probably be obtained with some fine tuning. Range can be obtained by reducing large number Ln into a small number Ln added to an easily computer constant. The opposite applies for small Lns which can come from a larger Ln and subtraction of an easily computed constant. The idea is to push the Ln performed into optimum range for the algorithm by shifting it with the constant.
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
Too bad negative armor is capped to a very small range, hence why LnAprox works. No need for it to support very large numbers since there are no very large numbers in this application?

Why should anyone use an implementation thats inferior in every relevant aspect? LnAprox might work for this special application, but with Math.ln it will work faster and with higher accuracy.

And even for the very small range of 0 < x < 1, LnAprox fails already for values like 0.001 (all digits wrong) vs Math.ln (first 7 digits correct).

Additional accuracy could probably be obtained with some fine tuning. Range can be obtained by reducing large number Ln into a small number Ln added to an easily computer constant.

Sure, but a fine tuned and more efficient version already exists. No need to reinvent the wheel ;)
 
Last edited:
Level 23
Joined
Aug 1, 2013
Messages
4,657

JASS:
function TriggerRegisterUnitEventEx takes trigger whichTrigger, unit whichUnit, unitevent whichEvent returns event
	if whichEvent == EVENT_UNIT_DAMAGED then
		// Save whichTrigger handle in a data structure
	endif
	return TriggerRegisterUnitEvent(whichTrigger, whichUnit, whichEvent)
endfunction

// ...
hook TriggerRegisterUnitEventEx before TriggerRegisterUnitEvent // Example syntax

That should be enough to do the trick...

That would be horrible.
The regular action where the custom function is hooked on will still run.
So when you use TriggerRegisterUnitEventEx(), you will then create the event 2 times.

However, why did noone mention that you only have to disable one trigger?
The trigger that is meant for the unit that takes damage.
So we would save that trigger in a trigger array under the index of the custom value of the unit using a Unit Indexer.
This will allow us to directly disable the specific trigger that we need.
It is just a problem how to null the trigger in the array again.

However using FirstOfGroup iterations and 30s to 300s reset intervals, you have 0 noticeable performance loss.
I mean, I tested with the FirstOfGroup iteration on 4k units and it did the job in a blink of an eye.
And you are not creating enough units in 30 seconds to make it fail.

um listen there is no need to make this compatible with dds's and shield systems and what not this is a system for small maps, if youre doing a big project then you should go with a custom combat system ( where armor, damage ,armor type ,damage type, shields ,healing etc are handled by the system and not the game engine )
I understand your point and I too think that big maps need something better than regular WC3 systems.
However that doesnt mean that small maps cannot have a DDS.
Small maps + DDS = no GetUnitArmor ... wut?
 
Level 13
Joined
Dec 12, 2012
Messages
1,000
That would be horrible.
The regular action where the custom function is hooked on will still run.
So when you use TriggerRegisterUnitEventEx(), you will then create the event 2 times.

Uhm, no ;)

I think you misunderstood some things here. First this is all hypothetic because there is no hook a before y in vJass. Second a function like TriggerRegisterUnitEventEx would of course be private and not accessible by users. The whole point after all is to not use any Ex function but the native directly. And third, correct implemented hooks would of course not replace the hooked function within the hook itself since that would cause an infinite loop anyway.

However, why did noone mention that you only have to disable one trigger?
The trigger that is meant for the unit that takes damage.
So we would save that trigger in a trigger array under the index of the custom value of the unit using a Unit Indexer.
This will allow us to directly disable the specific trigger that we need.

Of course its possible to do this manually. But thats not the point.

The whole point of the discussion was, to have a general way to disable events. Your method doesn't help if you use a library that doesn't care about your trigger array but uses its own internal triggers.

However that doesnt mean that small maps cannot have a DDS.
Small maps + DDS = no GetUnitArmor ... wut?

+1 on that point.
 
Level 34
Joined
Sep 26, 2009
Messages
8,435
Isn't armor an integer? At least an approximation to integer could be useful, right?

I have a concept that you could deal damage from one unit to the other, reduce their armor by 1 or increase by one until the damage doesn't get reduced. That value can then represent the approximate armor
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
not possible to release full source code thanks to licensing of certain libraries they used to build this game.
We dont know how much they made themselves and how much they bought, maybe even the whole pathfinding system is proprietary and you know, whats the point of having wc3 if you dont have pathfinding system? (thats just example)
 

Dr Super Good

Spell Reviewer
Level 57
Joined
Jan 18, 2005
Messages
26,447
not possible to release full source code thanks to licensing of certain libraries they used to build this game.
We dont know how much they made themselves and how much they bought, maybe even the whole pathfinding system is proprietary and you know, whats the point of having wc3 if you dont have pathfinding system? (thats just example)
They are using at least 2 proprietary licenced components.
  1. PKWare compression libraries. Used as part of the MPQ file system component.
  2. Intel Advanced Imaging libraries. At least the JPEG part is used as part of the BLP I/O component.
 
Level 12
Joined
Mar 13, 2012
Messages
1,121
So basically what should I do?
Burry it in a deep cave because it will always have potentially gamebreaking problems.

At least until someone finds a real way to get armor, not via this pseudo unit damaging thing.
 
Level 12
Joined
Mar 13, 2012
Messages
1,121
What a positive thought!
I did not post this to bring you or anyone down.
But if there exists a buggy resource, or even worse, a resource that sometimes bugs and where its next to impossible to sort out why it malfunctions for the poor guy implementing it, people should be aware of that.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
for some reason when I placed paladin down and gave him few rings, then kept changning base armor, the value(while it was positive) was always off by exactly 0.100.

There are many places where this will not work, as outlined by earlier posts, but I think this is still in good enough state to be approved(now that it actually calculates negative armors correctly), but I think the documentation should outline the catches where it wont work(surf the thread, I think mainly last 2 pages outline a lot of stuff).

I will wait for other mod's word on this before approving it.

My decision is based on merit that this is basically the closest you can get when it comes to getting armor, without resolving to memory reads, or getting native exposed, or doing all sorts of hacks for all the quirks.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
thats actually very possible :D

Any other Jass mods want to say what they think about approving this? I mean it works, even tho it may not work perfectly in all situations, but the code size compared to the funcionality would explode massivly if we wanted to fix everything, and not everything is fixable, and while resources should be universal if possible, I think this is as close as it gets without some really really wierd shit going on
 
Top