Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dragging - Add hookable events #10304

Merged
merged 8 commits into from
Sep 11, 2024
Merged

Conversation

some-evil-kitty
Copy link
Contributor

@some-evil-kitty some-evil-kitty commented Sep 10, 2024

Set box velocity to unit velocity

When merged this pull request will:

  • Allow dropped objects to inherit the velocity of the unit carrying them. For instance, running and dropping a box while running will make the box continue forward.
  • Add public API, so that 3rd party mods can modify carrying/dragging behaviour

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

Set box velocity to unit velocity
Copy link
Contributor

@TheCandianVendingMachine TheCandianVendingMachine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't see why this wouldn't work, lgtm

Co-authored-by: PabstMirror <pabstmirror@gmail.com>
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm opposed to this.

From a physics point of view it makes sense, but I don't like this - it feels whack.
I feel this will allow for more Arma'ing, without providing an actual benefit: All carried items are invulnerable for 2 seconds after being released, as to avoid then taking unjustified damage (placing a carried item shouldn't destroy your box). With this PR, one could make a carried item into a guided and invulnerable projectile. The only thing that's preventing that is that, in most scenarios, is that players can't carry stuff whilst going faster than walking speed - which makes this change unnecessary for most gameplay scenarios anyway.

If you want to implement something like this, what would be better imo is to make public events for the dragging component. That way, 3rd party mods can edit some of the dragging mechanics as they please.

Before anyone undertakes any changes, can I get thoughts from the other ACE devs?

@some-evil-kitty
Copy link
Contributor Author

Sounds good to me (though the inspiration for this was in fact to turn it into a projectile, lol)

I'll shift it over to a local event shortly, which should allow any mod that wants to turn it into a projectile to do so.

@some-evil-kitty
Copy link
Contributor Author

Do we think dragging should also have an event while we're at it?

@johnb432
Copy link
Contributor

Do we think dragging should also have an event while we're at it?

Yes.

named in keeping with the func names
@some-evil-kitty some-evil-kitty changed the title Carrying - Allow dropped objects to inherit velocity Carrying - Add hookable events to dropping objects Sep 11, 2024
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo the events should be raised at the very end. It means that ACE couldn't overwrite anything you'd set: e.g. if you set the position of a dragged person in the event, it will be overwritten by ACE's behaviour (see line 67-70).
Any objections from others?

If you think you need a hookable event earlier for some reason, we can always add a second one (a pre and a post event).

EDIT: I've made a branch with the changes I had mind, see https://github.com/acemod/ACE3/tree/dragging-add-hookable-events.

@@ -46,6 +46,7 @@ if (_tryLoad && {!(_target isKindOf "CAManBase")} && {["ace_cargo"] call EFUNC(c
} else {
// Release object
detach _target;
[QGVAR(objectDroppedCarry), [_unit, _target]] call CBA_fnc_localEvent;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would place this inside the main scope and pass _loadCargo, so that 3rd party mods can intervene even if the item will be loaded into cargo.

@some-evil-kitty
Copy link
Contributor Author

@johnb432
Forgive my github noobishness, but I'm not sure how to yank that commit from your branch into this one. It's possible, correct?

@PabstMirror
Copy link
Contributor

merged in

@PabstMirror PabstMirror added this to the 3.18.0 milestone Sep 11, 2024
@LinkIsGrim LinkIsGrim dismissed johnb432’s stale review September 11, 2024 23:54

Stale, PR goal changed

@LinkIsGrim LinkIsGrim merged commit fef34a8 into acemod:master Sep 11, 2024
4 checks passed
@johnb432 johnb432 changed the title Carrying - Add hookable events to dropping objects Dragging - Add hookable events Sep 12, 2024
@johnb432 johnb432 added kind/feature Release Notes: **ADDED:** and removed kind/documentation labels Sep 12, 2024
@some-evil-kitty some-evil-kitty deleted the patch-1 branch September 12, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants