SkriptLang / Skript

Skript is a Spigot plugin which allows server admins to customize their server easily, but without the hassle of programming a plugin or asking/paying someone to program a plugin for them.
https://docs.skriptlang.org
GNU General Public License v3.0
1.08k stars 370 forks source link

Make Right click event open to either hand #6465

Open JakeGBLP opened 9 months ago

JakeGBLP commented 9 months ago

Suggestion

Skript tracks the click events to avoid calling a click event with both hands at the same time, i think there should be a way to listen to both at the same time without the use of a function or a loop (looping both held items)

@sovdeeth mentioned that something like:

on right click with diamond axe with both hands:

is ambiguous, which I agree with, and opted for something like

on right click with diamond axe with either hand:

adding both wouldn't hurt in my opinion but if one must be chosen the the latter is the way to go about it

Why?

to explain it better, here's a dumbed down version of what I'm doing, with and without the change I'm suggesting

# ------ WITH Both Hands Support ------

on right click with both hands:
  set {_i} to item
  {_i} is diamond axe
  set {_offset} to 0.4 if hand is main hand, else -0.4
  # idk how to phrase this better
  # the point of this line is to spawn the entity near the hand that is throwing the axe
  # so it's worth mentioning that in my actual code I'm also checking if the player is right or left handed to set this offset
  # I won't do that here since it's not relevant for the sake of explaining this
  set item in event-hand to air
  # idk how to phrase it better
  # this could perhaps be changed to 'set slot event-hand of player to air' or to 'set event-hand to air'
  # not that relevant in this case
  set {_v} to vector in player' direction
  spawn item display {_offset} to the right of player's eyes:
    set display item of entity to {_i}
    set {_e} to entity
  loop 100 times:
    teleport {_e} to location of {_e} ~ {_v}
    wait tick
  kill {_e}
  give player {_i}

# ------ WITHOUT Both Hands Support ------

on right click:
  set {_v} to vector in player' direction
  set {_tools::*} to tool of player and offhand tool of player
  loop {_tools::*}:
    set {_i} to loop-value
    {_i} is diamond axe
    if {_i} is tool of player:
      set {_offset} to 0.4
      # same thing as earlier
      # not checking which hand is the actual main hand as it's not relevant here
      set tool of player to air
    else if {_i} is offhand tool of player:
      set {_offset} to -0.4
      set offhand tool of player to air
    spawn item display {_offset} to the right of player's eyes:
      set display item of entity to {_i}
      add entity to {_e::*}
  loop 100 times:
    loop {_e::*}:
      teleport loop-value-2 to location of loop-value-2 ~ {_v}
    wait tick
  kill {_e::*}
  give player 2 of {_i}

to make the same thing it becomes necessary to add 2 loops

and this doesn't even take into account the fact that, in the WITHOUT example, since we're creating a sort of projectile axe, how would you even handle killing the display item and giving back the item? because those 2 entities need to have separate collision checks to make that happen, so that would be another problem that would be easily avoided if support for both hands was added

image

Other

worth mentioning that this might required #6207 to be completed, not sure about this

Agreement

JakeGBLP commented 9 months ago

Sovde also mentioned that this can be solved with a function, and provided the following snippet:

on right click:
    # check main hand
    if tool of player is a diamond axe:
        throwAxe(player, (player's tool), 0.4)
        set player's tool to air

    # check off hand
    if offhand tool of player is a diamond axe:
        throwAxe(player, (player's offhand tool), -0.4)
        set player's offhand tool to air

function throwAxe(thrower: entity, item: item, offset: number = 0):
    set {_v} to vector in {_thrower}'s direction
    spawn item display {_offset} to the right of {_thrower}'s eyes:
        set display item of entity to {_item}
        set {_e} to entity
    loop 100 times:
        teleport {_e} to location of {_e} ~ {_v}
        wait tick
    kill {_e}
    give {_item} to {_thrower}

which does solve the issue, but in my opinion just doesn't feel natural

sovdeeth commented 9 months ago

6415 potentially related.

In my opinion, this isn't entirely necessary, as it's easy enough to deal with this manually. However, I think taking a closer look at how exactly we want EvtClick to function is a good idea. The current solution is a leaky bandaid, if you'll forgive my mixture of metaphors.

Looking at it with fresh eyes and the knowledge of modern minecraft mechanics and re-evaluating how much control we want to give users is a good idea, in my opinion.