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

Add reload mutex as canInteractWith condition #4062

Merged
merged 5 commits into from
Jul 9, 2016

Conversation

commy2
Copy link
Contributor

@commy2 commy2 commented Jul 7, 2016

When merged this pull request will:

@commy2 commy2 added the kind/bug-fix Release Notes: **FIXED:** label Jul 7, 2016
@commy2 commy2 added this to the 3.6.1 milestone Jul 7, 2016
@nicolasbadano
Copy link
Contributor

Hey, hold on. GVAR(isReloading) doesn't exist yet. It's currently called GVAR(ReloadMutex) and it is defined only on the gestures module. IMO it should be moved to common and renamed.

Also I wonder if it would be possible to set the variable from an AnimChanged EH instead of actionKeys for better reliability and joystick support.

@commy2
Copy link
Contributor Author

commy2 commented Jul 7, 2016

AnimChanged or any other anim event handler don't trigger for gestures like the reload animations.

@nicolasbadano
Copy link
Contributor

AnimChanged or any other anim event handler don't trigger for gestures like the reload animations.

Too bad; I was half expecting that reply though.

@jonpas jonpas changed the title add reload mut ex as canInteractWith condition add reload mut exas canInteractWith condition Jul 7, 2016
@jonpas jonpas changed the title add reload mut exas canInteractWith condition Add reload mutex as canInteractWith condition Jul 7, 2016
@commy2
Copy link
Contributor Author

commy2 commented Jul 7, 2016

^ this is my version of at least supporting the modifier keys. Not finished, because:

_a = -1207959552; (_a + 1) in [_a]
-> true

FML

@jonpas
Copy link
Member

jonpas commented Jul 7, 2016

Note: still needs additional canInteract checks on keybinds in all affected components.

@commy2
Copy link
Contributor Author

commy2 commented Jul 7, 2016

No, it's a canInteractWith condition.

@jonpas
Copy link
Member

jonpas commented Jul 7, 2016

Yes, whatever it's named. :P

@nicolasbadano
Copy link
Contributor

Yes, whatever it's named. :P

Nah, @jonpas . @commy2 means that this works automatically, without having to add any extra checks

@commy2
Copy link
Contributor Author

commy2 commented Jul 7, 2016

I think it's impossible to work with these offsets:
(-1207959552 + 32) + 1207959552
0

@nicolasbadano
Copy link
Contributor

It's a precision issue; the numbers are being treated as floating point

@commy2
Copy link
Contributor Author

commy2 commented Jul 7, 2016

Yeah, but that means that actionKeys cannot be used for this - at least not when we want to support modifier keys.

@nicolasbadano
Copy link
Contributor

nicolasbadano commented Jul 7, 2016

at least not when we want to support modifier keys.

Let's not support it. I doubt anyone uses one for reloading. We don't support joysticks anyway. The worst thing it can happen is that we retain current behaviour, right?

@jonpas
Copy link
Member

jonpas commented Jul 7, 2016

Nah, @jonpas . @commy2 means that this works automatically, without having to add any extra checks

Ah, in that case - awesome!

@commy2
Copy link
Contributor Author

commy2 commented Jul 7, 2016

I don't like how easy it is to trick the reload mutex.

Pressing R in a reloading incompatible stance starts the timer and when then actually doing the reloading, the isReloading value is reset mid reload.

@nicolasbadano
Copy link
Contributor

Yeah, it's pretty unreliable stuff.

incompatible stance

Those can be detected from config I guess, right?

@PabstMirror
Copy link
Contributor

Current interaction conditions take args [_player, _target],
so you could do something like check if another player can interact with you.

This new condition would only be valid for ACE_player,
but I really don't think this is a problem just wanted to mention it.

@commy2
Copy link
Contributor Author

commy2 commented Jul 8, 2016

Same with the old one. In the end it's just a hacky work around. : /

@CorruptedHeart
Copy link
Member

CorruptedHeart commented Jul 8, 2016

I assume this event is fired too late? Reloaded and can't be used in any way?

@commy2
Copy link
Contributor Author

commy2 commented Jul 8, 2016

Reloaded is fired when the animation ended. too late...

I even thought about SoundPlayed event handler, but the only noise the player makes that it can't pick up is reloading it seems ...

@commy2 commy2 removed the status/WIP label Jul 8, 2016
@commy2
Copy link
Contributor Author

commy2 commented Jul 8, 2016

Should be good now.

Question: Do we want this as general canInteractWith condition, or would we be better of by if we add it to Finger / Advanced Throwing / Wipe Goggles directly?

@jonpas
Copy link
Member

jonpas commented Jul 8, 2016

I'd say general is fine.

@nicolasbadano
Copy link
Contributor

@commy2, does this trigger when reloading vehicles?

@commy2
Copy link
Contributor Author

commy2 commented Jul 8, 2016

It triggers whenever you press the reload key while being in the mission display.

@jonpas
Copy link
Member

jonpas commented Jul 9, 2016

Merge @commy2 ?

@commy2 commy2 merged commit d249132 into apex Jul 9, 2016
@commy2 commy2 deleted the disable-interaction-menu-while-reloading branch July 9, 2016 19:03
@commy2 commy2 self-assigned this Jul 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-fix Release Notes: **FIXED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants