1. Updated Resource Submission Rules: All model & skin resource submissions must now include an in-game screenshot. This is to help speed up the moderation process and to show how the model and/or texture looks like from the in-game camera.
    Dismiss Notice
  2. DID YOU KNOW - That you can unlock new rank icons by posting on the forums or winning contests? Click here to customize your rank or read our User Rank Policy to see a list of ranks that you can unlock. Have you won a contest and still havn't received your rank award? Then please contact the administration.
    Dismiss Notice
  3. The Lich King demands your service! We've reached the 19th edition of the Icon Contest. Come along and make some chilling servants for the one true king.
    Dismiss Notice
  4. The 4th SFX Contest has started. Be sure to participate and have a fun factor in it.
    Dismiss Notice
  5. The poll for the 21st Terraining Contest is LIVE. Be sure to check out the entries and vote for one.
    Dismiss Notice
  6. The results are out! Check them out.
    Dismiss Notice
  7. Don’t forget to sign up for the Hive Cup. There’s a 555 EUR prize pool. Sign up now!
    Dismiss Notice
  8. The Hive Workshop Cup contest results have been announced! See the maps that'll be featured in the Hive Workshop Cup tournament!
    Dismiss Notice
  9. Check out the Staff job openings thread.
    Dismiss Notice
Dismiss Notice
60,000 passwords have been reset on July 8, 2019. If you cannot login, read this.

[vJASS] UnitIssueOrder

Discussion in 'Triggers & Scripts' started by Krogoth, May 6, 2013.

  1. Krogoth

    Krogoth

    Joined:
    Apr 5, 2011
    Messages:
    247
    Resources:
    0
    Resources:
    0
    We have IssueImmediateOrder, IssuePointOrder and IssueTargetOrder.
    Is it good if we would have this:
    Code (vJASS):
    function IssueOrder takes unit u, integer id, real x, real y, unit target returns nothing
    Is it good in point of "clean progamming" to do IssueOrder(u, STOP, 0, 0, null) if we want stop, IssueOrder(u, MOVE, x, y, null) if we want move, IssueOrder(u, ATTACK, 0, 0, target) if we want attack. It removes a lot of crap conditions and shorters code. (Really, Blizzard made a lot of functions with 5132850708 parameters useless in most of cases)
     
  2. Dr Super Good

    Dr Super Good

    Spell Reviewer

    Joined:
    Jan 18, 2005
    Messages:
    25,545
    Resources:
    3
    Maps:
    1
    Spells:
    2
    Resources:
    3
    IssueOrder(u, STOP, 0, 0, null)
    [0,0] is a valid point. It usually is the exact middle of the play field. Worse still is [0,0] is what all invalid positions point at (such as the position of null unit) so often a lot of garbage gets sent there.

    It is bad programming practice since you have limited scope control and bad argument coupling. Their current way is the best since it avoids passing unnecessary arguments (no argument coupling) and allows you to explicitly define what is being targeted.

    An example being... Are you using "patrol" a unit (follow) or the ground (patrol)? Currently both use separate natives to invoke but your suggestion would make it impossible for the game to distinguish between the two.
     
  3. Krogoth

    Krogoth

    Joined:
    Apr 5, 2011
    Messages:
    247
    Resources:
    0
    Resources:
    0
    Is it so really important? 2-3 arguments? And it removes conditions as were said, so I am not even sure what exactly is efficient.
    Ofc no.
    If I need to patrol ground, I type:
    Code (vJASS):
    call IssueOrder(unit, PATROL, x, y, null)

    but if I need to patrol unit:
    Code (vJASS):
    call IssueOrder(unit, PATROL, <ANYTHING>, <ANYTHING>, target)

    Here is how I differentiate it:
    Code (vJASS):
        function IssueOrder takes unit u, integer id, real x, real y, widget target returns nothing
            if     Orders.Type[id] == ORDER_TYPE_IMMEDIATE then
                call IssueImmediateOrderById(u, id + ORDERS_OFFSET)
            elseif Orders.Type[id] == ORDER_TYPE_POINT then
                call IssuePointOrderById(u, id + ORDERS_OFFSET, x, y)
            elseif Orders.Type[id] == ORDER_TYPE_TARGET then
                call IssueTargetOrderById(u, id + ORDERS_OFFSET, target)
            elseif Orders.Type[id] == ORDER_TYPE_SMART then
                if target == null then
                    call IssuePointOrderById(u, id + ORDERS_OFFSET, x, y)
                else
                    call IssueTargetOrderById(u, id + ORDERS_OFFSET, target)
                endif
            endif
        endfunction


    Added:
    Without this function I need to write this ^ garbage every time I work with orders.

    So, num(+) > num(-), or not?
     
    Last edited: May 6, 2013
  4. Dr Super Good

    Dr Super Good

    Spell Reviewer

    Joined:
    Jan 18, 2005
    Messages:
    25,545
    Resources:
    3
    Maps:
    1
    Spells:
    2
    Resources:
    3
    You only need that "garbage" when using some orders. Most orders expect a specific target.

    As for efficiency...
    You pass unused arguments. This is called argument coupling and is often frowned upon in programming.
    You run unnecessary tests. There is no need to test if an order is point or target or smart as an order either is or is not and that is known during development.

    It does have some merits though. It could be used to support dynamic order systems where the exact nature of an order is not know during development (varies depending on game state). Then you need to check what sort of order it is as you may have to cope with an order of an unknown kind. An example would be for a role play map to get a unit to automatically issue orders.

    The biggest problem is that orders target widget not unit. This means your approach will be unable to cope with orders targeting items and destructible such as ordering a unit to pickup an item or ordering a worker to harvest from a tree.
     
  5. Krogoth

    Krogoth

    Joined:
    Apr 5, 2011
    Messages:
    247
    Resources:
    0
    Resources:
    0
    Y, i fixed already.
     
  6. Zwiebelchen

    Zwiebelchen

    Joined:
    Sep 17, 2009
    Messages:
    6,791
    Resources:
    12
    Models:
    5
    Maps:
    1
    Spells:
    1
    Tutorials:
    1
    JASS:
    4
    Resources:
    12
    Also, it requires to register all order strings that exist in the map manually.
    Plus, it doesn't work as soon as channel is involved, as channel allows for order strings to be used for any kind of order.

    It's only useful for those general orders like move, patrol, attack, etc..