Detachments v1.2.0.0

[Moderator]

00:01, 23rd Jun 2013
PurgeandFire: Changes made. Approved and recommended. A solid, unique system.

 

[.OmG.]

Sure that it could be useful.

If the leader dies, his/her role is taken over by the first member available.

I think it would more realistic if at deaths door of a leader, control will not take over.

 

[Garfield1337]

I think it would more realistic if at deaths door of a leader, control will not take over.

But what should happen with the detachment then? It can't be controlled without a leader.

 

[Almia]

you should link the Libraries with the /**/ parse.

call IssueImmediateOrder(.me, "stop")
set .orderTarget = null
set .orderId = OrderId("stop")

->

call IssueImmediateOrderById(.me, 851972)
set .orderPoint = false
set .orderId = 851972

http://www.hiveworkshop.com/forums/jass-resources-412/repo-order-ids-197002/

The number of ticks or "I" has can be configurable as well.
You can move the bar variable into a global

 

[Garfield1337]

you should link the Libraries with the /**/ parse.

What's that? ;o

order string -> order id

The performance boost opposed to improved readability of using orders in string rather than integer form is negligible. You can more easily see or change the issued order if it's in string form.

The number of ticks or "I" has can be configurable as well.
You can move the bar variable into a global

Well, bar fill speed is configurable so changing bar length shouldn't be needed.
But okay, I'll add that.

 

[Almia]

library Hello/*
*
*    */uses/*
*    */SomeLib/* somelib.com
*/

how about cacheing the order id with a variable? its readable : constant integer stop = 851972 or just create a global Lib for OrderIds which i linked.

 

[deathismyfriend]

U can use static ifs so that the library is optional also. This will give the user more flexibility.

 

[mckill2009]

- Pls make the RegisterPlayerUnitEvent NOT optional, the onInit is not good imo
- Put API methods at documentation
- Why you didnt use GetUnitGoldCost and GetUnitWoodCost?
- Cache your PlayerState and alike via local variables
- SimError notices be configurable

 

[Garfield1337]

- Pls make the RegisterPlayerUnitEvent NOT optional, the onInit is not good imo

Why would it not be optional? it gives user more freedom.
And what's wrong with onInit? I have to initialize the events.

- Put API methods at documentation

The public functions are already in the API.

- Why you didnt use GetUnitGoldCost and GetUnitWoodCost?

Training unit through reinforcement doesn't necessarily have to use object editor values. The user might want to train them a unit using object editor cost, separately from this system.

- Cache your PlayerState and alike via local variables

I'm using them twice only each in a function, is it worth creating a local variable for that?

- SimError notices be configurable

Ok, I'll add this.

 

[mckill2009]

Why would it not be optional? it gives user more freedom.
And what's wrong with onInit? I have to initialize the events.

Yeah but since you have many requirements atm, might as well include that...

I'm using them twice only each in a function, is it worth creating a local variable for that?

Even if 2, it's still slower coz total is (player+state)*2 = 4, while caching does it twice...also I'm suggesting this for readability's sake...

EDIT:
also I didnt see the libraries until now, why are you making a group library without anything in it except the group alone or one variable only?, and why dont you put the Z inside the code itself and the group?

EDIT 2:
you could use Nesthauru's TimerTolls for period frequency in it...

 

[Garfield1337]

Yeah but since you have many requirements atm, might as well include that...

It is included, but it's optional. Why would you want it to be more limited?

Even if 2, it's still slower coz total is (player+state)*2 = 4, while caching does it twice...also I'm suggesting this for readability's sake...

What's "player+state"? The GetPlayerState function repeats once for each resource. It's very readable atm because of intuitive names of these functions; using local would add an unecessary line.

EDIT:
also I didnt see the libraries until now, why are you making a group library without anything in it except the group alone or one variable only?, and why dont you put the Z inside the code itself and the group?

Because if every library used its own global group or Z functions, you'd have unnecessary repetitions of these. Most systems or maps have a small library with Z functions, if they're present, the one used by this library can just be removed. Same goes for group, some libraries like GroupUtils have their own global group for such uses, so instead of importing GroupUtils, I added this group. If there's another library that has such group, the one used here can be removed.

EDIT 2:
you could use Nesthauru's TimerTolls for period frequency in it...

I'm not sure what TimerTools is for. The idea of merging timers with similar timeouts seems wrong to me. I'd rather have exact periods.
The resource I use for periodic iterations is something between T32 and CTL.
I didn't use CTL because it has what appears to me as many unneeded things.

 

[edo494]

I must ask

*        */ II                      /*
*        */ Group                   /*
*        */ Z                       /*

what are those libraries? they have no links to them, so I wonder

 

[deathismyfriend]

the onInit method is fine. not sure y it wouldnt be.

i dont like how ur struct doesnt extend an array. it produces a lot more code when u dont extend an array. so its less effecient and slower

 

[Garfield1337]

I must ask

*        */ II                      /*
*        */ Group                   /*
*        */ Z                       /*

what are those libraries? they have no links to them, so I wonder

They aren't public libraries. Group provides a global group for enumeration (so I don't have to recreate it each time), Z provides functions such as GetUnitZ, SetUnitZ and GetPointZ and II is used for periodic instance iteration, something like T32 or CTL.

the onInit method is fine. not sure y it wouldnt be.

i dont like how ur struct doesnt extend an array. it produces a lot more code when u dont extend an array. so its less effecient and slower

Meh, it probably would be better if it extended an array, but I don't think it matters that much. The main difference is, as far as I know, allocation method. Since the instances aren't allocated periodically or something like that, I don't think the efficiency beats readability and intuitivity here.

 

[deathismyfriend]

nestharus made a page on it to show the differences and it is a big deal. it deals with the allocation and deallocation of structs.

 

[edo494]

if they are not public you should provide the code of those, not everyone will download the map just to get 3 libraries, thats why we have vJass, so we dont have to download all maps to check code or copy it, unlike GUI

 

[Garfield1337]

Alright, I'll add it.

EDIT: There's also a reason why I can't make these structs extend array. The AutoData module from AutoIndex, used in these structs, doesn't work when structs extend array.

 

[deathismyfriend]

y doesnt it work for that ? ive never used the AutoData module b4. extending array should not cause problems tho.

 

[mckill2009]

It is included, but it's optional. Why would you want it to be more limited?

it's not being limited, I can see also your Z, II and Group is not optional, why the Register is an exception?, now you've mentioned unecessary lines, well that is one, coz we have the register library already ...

What's "player+state"? The GetPlayerState function repeats once for each resource. It's very readable atm because of intuitive names of these functions; using local would add an unecessary line.

GetPlayerState(p, PLAYER_STATE_RESOURCE_GOLD), calls the (player and gold), then you did it again so 2*2=calls 4, unecessary lines doesnt hurt as much as unecessary width...

Because if every library used its own global group or Z functions, you'd have unnecessary repetitions of these. Most systems or maps have a small library with Z functions, if they're present, the one used by this library can just be removed. Same goes for group, some libraries like GroupUtils have their own global group for such uses, so instead of importing GroupUtils, I added this group. If there's another library that has such group, the one used here can be removed.

You dont need Group Utils, in fact also, bj_lastCreatedGroup can do, imo, it's a waste of importing libraries with that few code inside...also, moderators tend to approve systems with approved system resource...

 

[PurgeandFire]

Overall I like this system. It is creative. And by the way, ty for the awesome test map. I actually messed around with it for a while before I realized I was supposed to be moderating a resource. Here is my review:

Overall, this is a solid system. It has nice checks (meaning, it checks things for actual playable maps), great error messaging, and the code is very neat. The improvements I am going to suggest will be mostly minor.

• In static method filter, you need to null f.
• This following method is unorthodox but it is a nice little tip:
  

  if GetLocalPlayer() != GetOwningPlayer(.me) then
                      call SetTextTagVisibility(.tag, false)
                  endif

  Can become:
  

  call SetTextTagVisibility(.tag, GetLocalPlayer() == GetOwningPlayer(.me))

  Of course, it is a miniscule difference, but it looks pretty clean. However, if you would like to keep the prior method for readability purposes, that is completely fine.
• For method iterate1, you may be able to take advantage of IsUnitInRangeXY() to eliminate some lines and eliminate the SquareRoot/multiplication:
  

  * * * * method iterate1 takes nothing returns nothing
  * * * * * * //This method is executed every second and is used to check if a member wanders far off
  * * * * * * local Soldier soldier = .first //Starting for first soldier in the list
  * * * * * * local real lx = GetUnitX(.me)
  * * * * * * local real ly = GetUnitY(.me)
  * * * * * * local real a
  * * * * * * loop
  * * * * * * * * exitwhen soldier == 0 //When hitting the end of the list, the loop finishes
  * * * * * * * * if not IsUnitInRange(soldier.me, .me, WANDER_DISTANCE) then
  * * * * * * * * * * //If the member is too far, he/she is issued an order to go back
  * * * * * * * * * * set a = Atan2(GetUnitY(soldier.me) - ly, GetUnitX(soldier.me) - lx)
  * * * * * * * * * * set Soldier.checkOrder = false //To prevent infinite loop
  * * * * * * * * * * set soldier.orderId = OrderId("move")
  * * * * * * * * * * set soldier.orderPoint = true
  * * * * * * * * * * set soldier.orderX = lx + ARRIVAL_RANGE*Cos(a)
  * * * * * * * * * * set soldier.orderY = ly + ARRIVAL_RANGE*Sin(a)
  * * * * * * * * * * set soldier.orderTarget = null
  * * * * * * * * * * call IssuePointOrderById(soldier.me, soldier.orderId, soldier.orderX, soldier.orderY)
  * * * * * * * * * * set Soldier.checkOrder = true
  * * * * * * * * endif
  * * * * * * * * set soldier = soldier.next
  * * * * * * endloop
  * * * * endmethod

  (^Sorry for the stars, something retarded happened to my text editor.)
  
  Something along those lines. Untested, but I assume it would look something like that.
• In static method createFilter, you can just use return isLeader(u), unless you intended it to look like that for configuration purposes.
• The library "Group" is a bit of a superfluous requirement. It is good to take advantage of a single global group as much as you can, but I think making libraries such as that is best left for map making. For your own map, you can make it require "Group" but for a system I'd just inline it and make one global group. Two reasons: (1) it is daunting when you go to use a system and you see a bunch of triggers to copy and paste (2) it has no benefit compared to having a single global group unless the user uses other systems that use it--or if they use it themselves. Still, this is entirely up to you. I'm just giving some feedback to help promote your system.
• For II, it is a similar case. You may want to remove the versions that are not pertinent to this system (unless you plan on using that system for other spells/systems). You don't have to remove them completely, but make them regular comments instead of actual "//!" textmacro calls.

That's about it. Also, just a note: you don't have to use custom allocation (in regard to the posts before this).

@mckill: Optional is better than required. Extra dependencies is not necessarily a good thing (they usually aren't a bad thing either, but it is something to take in moderation). When you can make it optional without much of a hassle, then it is good to do so.

 

[Garfield1337]

Thanks for the great review and suggestions.

I have implemented them and updated the resource.

For the Group library, I have removed it and added a configurable variable. Currently, the library creates the group, but if there is a global group for dynamic enumerations, the user may assign it to the variable in the library.

 

[edo494]

the Z library looks quite interesting, I think you could submit it because it has the 3 very good function for Z coordinates. I think that it would be quite useful even tho its simple and short

The II looks like some period manipulator, it also looks quite interesting

 

[deathismyfriend]

it would be nice if u could post those things in ur pastebin right here so we can see them easier.

 

[Garfield1337]

I think there are many libraries with Z functions, some posted in small code snippets. It should be standardized at some point...
I don't know if II would be approved, public libraries like CTL and/or TimerTools are supposed to be used and are probably more efficient, but CTL only supports 1 period and TimerTools isn't designed to iterate structs. I'm using II because of its simple API and support for multiple periods.

it would be nice if u could post those things in ur pastebin right here so we can see them easier.

They are in pastebin. Links are in library requirements.

 

[edo494]

as long as it provides something new it would be approved(unless full of bugs and/or leaks), there are 3 damage detection systems, each of them does the same thing, but each of them does it in other way

I think this is not that bad, you can give it a try

 

[deathismyfriend]

I think there are many libraries with Z functions, some posted in small code snippets. It should be standardized at some point...
I don't know if II would be approved, public libraries like CTL and/or TimerTools are supposed to be used and are probably more efficient, but CTL only supports 1 period and TimerTools isn't designed to iterate structs. I'm using II because of its simple API and support for multiple periods.

They are in pastebin. Links are in library requirements.

ik they are in pastebin but i dont like looking at one system when its on multiple pages.

 

[mckill2009]

I'm using II because of its simple API and support for multiple periods.

Do you know that you can 'cheat' a timer period using CTL and T32?, just needs 2 extra real variable...

 

[Garfield1337]

Do you know that you can 'cheat' a timer period using CTL and T32?, just needs 2 extra real variable...

That's what II is doing, it uses a textmacro to create variables for every period needed.
Unless you mean something else, as I don't know a way with creating only extra 2 real variables.

 

[mckill2009]

That's what II is doing, it uses a textmacro to create variables for every period needed.
Unless you mean something else, as I don't know a way with creating only extra 2 real variables.

implement CTLExpire
   set .dur = .dur + 0.03125
   if .dur > 1.0 then
      set .dur = 0
      //run here
   endif
implement CTLEnd

I've used this method and seems no problem...

 

[Garfield1337]

That ruins the purpose of using CTL, II or T32 as it's inefficient (and not really accurate for every period, it works with 0.03125 because it's a factor of 1; for some other periods, value of dur would be above 1 when that if check is passed).

 

[mckill2009]

that's why I said 'cheat', if you want multiple periods, you may add multiple real instance also, instead of implementing multiple CTL's...

EDIT:
> is not really a big deal...

 

[deathismyfriend]

are u going to add those libraries to this page ?

 

[Garfield1337]

They aren't related to the system's code, only used by it. No reason to put it here, the external libraries are linked and each's code is on separate page

 

[Map Designer]

BEAUTIFUL,

You also have a spell formation of the troops? Like.
Captain: FORM A LINE
Footman: aye Captain
Captain: Form a Rectangle
Footmen: Aye Captain
Captain: Capture the Death Knight
Footman: FOR LORDEAROOOOOOOOOOOOOON!!!!!!!!!!!!! *SURRONDS DEATH KNIGHT*

This will be cool as well

 

[Fingolfin]

I really can't stand the neverending bigotry of the speed freaks, and other people advocating stuff that seriously damages readability for minuscule speed benefits. In systems like this performance is really, really not an issue, and i love that your code is so readable and well made.

That being said, there are some things outside of coding that i think you should change. First of all, your approach to replacing leader units breaks control groups. A better way would be to detect when the leader is dying, prevent it, and simply swap it with another random unit in the group. The swapped unit will take the leaders place and die, while the leader will have his HP set to that of the swapped unit. This also means the user doesn't have to add a bear form ability to each of the base units.

Second, although i do think using the ward classification is nifty, there are other ways to unable units to be selected. Here is what the Wc3c ability guide says about the locust ability:

Aloc (Locust): A unit that is given locust becomes unselectable, untargetable and ignores any pathing limitations. This means it can only be controlled and ordered with triggers. It won't show a life bar over its head too. Now to get rid of locust UnitRemoveAbility(u, 'Aloc') won't work therefore you'll need to chaos transform the unit into a different temporary unit type and immediately transform it back. However if the unit had been transformed with a chaos ability before then adding locust will make it unselectable but still targetable and it will also obey pathing rules. Another option to get rid of locust is the following hiding the unit, removing the ability and unhiding it again.

As you see, it is possible to use the locust ability, while still making the unit targetable and having it obey pathing rules.

Other than that, great system. Keep up the good work.

 

[Garfield1337]

The locust idea is great and would fit well with the way system works. If I find some time, I'll add it.
Thanks for suggestion.

 

[edo494]

link to depedency II is no longer valid, fix that please

 

[LordVandor1]

Thank youuu!
Thank you very much ^_^
I was looking for this for two hours but finally I found it!
It's the best trigger ever made!
With this I can do Medieval: Total War for Warcraft III!!!

 

[LordVandor1]

Okay, Warcraft : Total War is going really good,just...
I need commands to make the units use another formation,because my Generals are always the first to die (it's normal,they are in the first line!), even if they have 100 more hp than the Soldiers :/
Also,it's good that I can define the number of Soldiers per type of Detachment,but
I would also put that you can define how much space there is between troops in the formation...yes,for each Detachment.

Also,how come it isn't calculated when I put Commanders on the map with the editor instead of recruiting them???

 

[Garfield1337]

Im currently unable but ill hopefully update this soon to fit your needs.

 

[Vincent_Freeman]

I didn't get it. I've copied the whole system into a map. I changed your footmen and archers, and everything was ok. Then I created another commander and unit (with all spells and styff). I haven't done any scripts, so it was still ok. This new troops weren't working, wich is logical at this stage.
Then I created the variables, you've mentioned in this theme. I made them array (was that a mistake?) Copied the custom srcipts, put them after setting [1] variables. Now JNPG doesn't want to save my map and finds some mistakes.
Could you please and some screenshots to show how and where exactly do i need to put new variables? How custom units-commanders should look like in GUI?
Hope to hear from you soon.

 

[Garfield1337]

Your new units need to fit the description given in the How-to-use section of the code.
You get errors because you use an array, which isn't really needed and has different syntax from what is used in custom script.
Just make the variables listed in the variable editor. Then in an initialization trigger, set the values for these variables according to your new units and call the custom script right after.

 

[Vincent_Freeman]

Thank you) I corretced mistakes, now everything is working)
This little challenge makes me feel like a real coda
Your system is perfect, but are you going to make some map using it?
Another side question - how could possibly AI use this system?

 

[Garfield1337]

There is a map using this, it was for a techtree competition in my awards

You'd have to make custom AI to make it use this.

 

[Vincent_Freeman]

There is some problems. After a fight, where detachments' leaders died several times I got this picture. Orcs in the red square don't move at all and don't follow any leaders, Trolls in the blie square are don't know whom to follow - orc commanders or their troll commanders.

 

[Garfield1337]

Are you using the latest version? I've seen the problem before, but I thought I fixed it.

 

[Vincent_Freeman]

Yes, since you haven't uploaded the resource since may, the version nust be the latest.

 

[Pinzu]

There is a bug with the detachment training of captain. If you have a full detachment I can still queue up 4 units it will reach full training time and then complain correctly that I can't train, however for the archer captain this option is not posible, it will complain as soon as I try to trian more then what is posible.

Could you make the squad units unselectable?

 

[Hazop / Sarven]

It dosent work....
Every time I copy past the "Detachments" folder thingy and I save, it fails on me saying all of the triggers have been disabled and world edit crashes on me.