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 'CBA_fnc_addPlayerEventHandler' #328

Merged
merged 4 commits into from
May 16, 2016
Merged

add 'CBA_fnc_addPlayerEventHandler' #328

merged 4 commits into from
May 16, 2016

Conversation

commy2
Copy link
Contributor

@commy2 commy2 commented May 14, 2016

Port (?) from ACE.
This is a framework to add event handlers based around properties of the players avatar that are pretty usefull, but unfortunately require polling.

Possible events:
- "unit" - player controlled unit changed
- "weapon" - currently selected weapon change
- "loadout" - players loadout changed
- "vehicle" - players current vehicle changed
- "turret" - position in vehicle changed
- "visionMode" - player changed to normal/night/thermal view
- "cameraView" - camera mode changed ("Internal", "External", "Gunner" etc.)
- "visibleMap" - opened or closed map

@commy2 commy2 self-assigned this May 14, 2016
@commy2 commy2 removed the WIP label May 14, 2016
@nicolasbadano
Copy link
Contributor

nicolasbadano commented May 14, 2016

Mmm, I celebrate that these are getting added to CBA. However, I'm not really fond of the way it's executed for two reasons:

1- Most of the events are trivially cheap to evaluate, but instead of adding them to a single loop that always run (like in ACE), each one is tucked in it's own EH function, making them slower. This is specially true if you want to use many or all of them (like in ACE). I would instead recommend that either all of them are installed by default, or all are installed when the first is.

2- That a new installer function is required CBA_fnc_addPlayerEventHandler instead of CBA_fnc_addLocalEvent

@commy2
Copy link
Contributor Author

commy2 commented May 15, 2016

All checks could certainly be moved in one EachFrame event.

I don't want to have this loop running by default though. Most mods won't make use of any of these, so I want to avoid being critiqued for adding 'pointless' checks every frame. Imagine a community using CBA only for JR and CUP (Life + CUP) or even a mil sim comm with only CBA + TFAR.
A wrapper would still be needed in that case and I don't really see a problem with that.

@bux
Copy link
Contributor

bux commented May 15, 2016

How about running everything in one loop and have developers "opt-in" to the events?

@PabstMirror
Copy link
Contributor

We could dynamically build up the polling function with compile.

@commy2
Copy link
Contributor Author

commy2 commented May 15, 2016

How about running everything in one loop and have developers "opt-in" to the events?

Using the CBA_fnc_addPlayerEventHandler function once would be a way to opt-in I guess.

We could dynamically build up the polling function with compile.

Sure. That would mean compiling 8 functions at mission start though (because if you use all, they are all added after another)

I kinda like CBA_fnc_addPlayerEventHandler, because it gives these events context without having to name them playerInventoryChanged, playerWeaponChanged, ... Instead they can be called inventory, weapon etc.

@commy2 commy2 added this to the 2.3.2 milestone May 15, 2016
@nicolasbadano
Copy link
Contributor

Thanks @commy2 👍


Description:
Returns the currently controlled unit.
Different from "player" when remote controlling units via zeus.
Copy link
Contributor

Choose a reason for hiding this comment

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

Either shorten to one sentence or separate with empty line for possible future improvements to documentation generation.

@Killswitch00 Killswitch00 merged commit f1db069 into master May 16, 2016
@thojkooi thojkooi deleted the playerEvents branch May 16, 2016 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants