server icon indicating copy to clipboard operation
server copied to clipboard

[core, lua] Improve draw-in specification to fix some issues

Open TracentEden2 opened this issue 1 year ago • 7 comments

I affirm:

  • [x] I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • [x] I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • [x] I have read and understood the Contributing Guide and the Code of Conduct.
  • [x] I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

This PR changes the specification of draw-in functions in several places to support flexibility and the additional options needed for the draw-in logic of different mobs.

Current LSB draw-in problems:

  • The DRAW_IN mob mod is currently representing two different options that do not always occur together. Specifically, a DRAW_IN > 1 indicates alliance draw-in and a DRAW_IN > 1 also sets the draw-in trigger distance to the mob mod. Thus all mobs with a non-standard trigger distance are also forced to have alliance draw-in. This creates several draw-in bugs.
  • No ability to draw-in to the front of the mob (as opposed to mob center)
  • No ability to set a max draw-in distance (beyond which a player or alliance member will not be draw-in)
  • The lua draw-in function (used for ad-hoc draw-ins in special case mobs) does not allow any options and does not allow overriding the mob or hard coded draw-in defaults.

Changes by this PR:

  • Changes the DRAW_IN mob mod to be a bitmask mod that represents several different draw-in options including perform normal draw-in logic, include alliance members in draw-in, and draw-in to front of mob. Also adds mob mods MOBMOD_DRAW_IN_TRIGGER_DIST and MOBMOD_DRAW_IN_MAX_RANGE to allow specifying the draw-in trigger distance and the max draw-in range. The bitmask approach allows flexibility and easy future expansion to new draw-in types that LSB is still missing.
  • Changes the lua draw-in function to allow options as parameters. The order of precedence is thus: specified function option -> mob mod(s) -> hardcoded default option
  • Changes mobs with draw-in to use the new and updated draw-in mob mods

Steps to test these changes

Fight mobs with draw-in and vary the draw-in mob mods to set different options, also test the lua drawIn function call with different parameters

TracentEden2 avatar Jul 14 '24 19:07 TracentEden2

Why not change draw in to mean 1 -> target player + pets, and 2 for party/alliance, (so no numbers over 2) and add another mobmod for draw in trigger distance, without the need of having 4 local variables?

And actually, the original draw in could be changed to set draw in type (player or party / to in front of mob OR to specified locations, like Khaimaira)

Xaver-DaRed avatar Jul 14 '24 19:07 Xaver-DaRed

Why not change draw in to mean 1 -> target player + pets, and 2 for party/alliance, (so no numbers over 2) and add another mobmod for draw in trigger distance, without the need of having 4 local variables?

And actually, the original draw in could be changed to set draw in type (player or party / to in front of mob OR to specified locations, like Khaimaira)

The idea is there are a lot of potential differences between different draw-ins so would need to have several more mob mods as the dimensions are at least: alliance, in front, trigger dist, max dist, type (leash vs. no leash), include dead/mounted, etc. For brevity not all of these are included in this PR, leash and others will come later on. Also the local variables make understanding mob draw-in logic easier to understand from just the mob file (without needing to look through other logic).

TracentEden2 avatar Jul 14 '24 20:07 TracentEden2

Since you are already reducing the original mobmod to a bit, one could use the original mobmod as a bitmask. original mobmod (type) bit 0 -> has draw in bit 1 -> include alliance (vs target only) bit 2 -> specified pos (vs in front) bit 3 -> include dead bit 4 -> include mounted bit 5 -> leash (i don even know what this is) ..and so on

New mobmod draw in trigger distance

This are just ideas. I'm personally not a fan of abusing the use of local variables for global systems

Xaver-DaRed avatar Jul 14 '24 20:07 Xaver-DaRed

Since you are already reducing the original mobmod to a bit, one could use the original mobmod as a bitmask. original mobmod (type) bit 0 -> has draw in bit 1 -> include alliance (vs target only) bit 2 -> specified pos (vs in front) bit 3 -> include dead bit 4 -> include mounted bit 5 -> leash (i don even know what this is) ..and so on

New mobmod draw in trigger distance

This are just ideas. I'm personally not a fan of abusing the use of local variables for global systems

Yeah, could maybe do it with a bitmask and only three new mob mods: trigger distance, max distance, and leash distance, however we lose the readability and explicitness and have to rely on potential code comments. In the end I do not mind either method, but I guess depends on what should be prioritized.

Leash draw-in is where the mob only allows the players to move a specified distance away from a fixed center point of a shape before drawing in potentially to a center point or other point in that shape, thus the mobs never leaves that shape (like dog on leash), this is like Aspid for example (note aspid also has normal non-leash draw-in it can use inside the circle)

TracentEden2 avatar Jul 14 '24 20:07 TracentEden2

Since you are already reducing the original mobmod to a bit, one could use the original mobmod as a bitmask. original mobmod (type) bit 0 -> has draw in bit 1 -> include alliance (vs target only) bit 2 -> specified pos (vs in front) bit 3 -> include dead bit 4 -> include mounted bit 5 -> leash (i don even know what this is) ..and so on

New mobmod draw in trigger distance

This are just ideas. I'm personally not a fan of abusing the use of local variables for global systems

Ok, I updated the PR to use a bitmask mob mod for DRAW_IN and two new mob mods for setting the trigger distance and max range :)

TracentEden2 avatar Jul 26 '24 23:07 TracentEden2

CI has a real complaint saying PTarget is not used in a lambda

WinterSolstice8 avatar Jul 28 '24 23:07 WinterSolstice8

CI has a real complaint saying PTarget is not used in a lambda

Thanks for pointing this out, I have updated the PR now!

TracentEden avatar Jul 29 '24 12:07 TracentEden

I don't think this PR is relevant anymore, since draw-in got moved into Lua? https://github.com/LandSandBoat/server/pull/6566

zach2good avatar Mar 31 '25 10:03 zach2good

I don't think this PR is relevant anymore, since draw-in got moved into Lua? #6566

Yep, I think better to add any remaining fixes from this PR in separate Lua PR.

TracentEden2 avatar Apr 01 '25 17:04 TracentEden2