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

Disablable NameTags Module option #1204

Merged
merged 16 commits into from Jun 2, 2015
Merged

Disablable NameTags Module option #1204

merged 16 commits into from Jun 2, 2015

Conversation

jonpas
Copy link
Member

@jonpas jonpas commented May 14, 2015

Because it's not there I guess. 😁

Let me know if I messed something up, my first ACE Settings/Modules job!

  • Add SettingsChanged EH
  • Figure out why module options doesn't work ("SCALAR" -> "NUMBER")

@@ -24,4 +26,6 @@ GVAR(ShowNamesTime) = -10;


// Draw handle
addMissionEventHandler ["Draw3D", {_this call FUNC(onDraw3d);}];
if (GVAR(showPlayerNames) > 0 || GVAR(showVehicleCrewInfo)) then {
addMissionEventHandler ["Draw3D", {_this call FUNC(onDraw3d);}];
Copy link
Contributor

Choose a reason for hiding this comment

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

If the GVAR(showPlayerNames) is initially zero and you turn it on later, they won't draw due to draw3d eh not being installed.

You need to monitor the "SettingsChanged" EH to check for that happening and add remove the Draw3D event accordingly. Check viewdistance for a possible implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

@nicolasbadano nicolasbadano added the kind/enhancement Release Notes: **IMPROVED:** label May 14, 2015
@nicolasbadano nicolasbadano self-assigned this May 14, 2015
@jonpas
Copy link
Member Author

jonpas commented May 14, 2015

Will add EHs tomorrow.

Conflicts:
	addons/nametags/ACE_Settings.hpp
class showPlayerNames {
displayName = "$STR_ACE_NameTags_ShowPlayerNames";
description = "$STR_ACE_NameTags_ShowPlayerNames_Desc";
typeName = "SCALAR";
Copy link
Contributor

Choose a reason for hiding this comment

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

should be number

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch:

typeName = "NUMBER" in modules equals typeName = "SCALAR" for variables

BIS consistency

Copy link
Member Author

Choose a reason for hiding this comment

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

Already fixed, will push with the rest of the stuff, thanks a lot @Glowbal for telling me this!

#BIS' fault

@jonpas
Copy link
Member Author

jonpas commented May 15, 2015

If all is good, this is done.

};

// Set the EH which waits for a setting to be changed, so that the effect is shown immediately
if (!GVAR(showPlayerNamesForce)) then {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not guaranteed to be set to the correct most up to date value on clients and JIP. Maybe make it so this if statement is inside the settingChanged EH?

Same goes for the lines 27/29.

Copy link
Member Author

Choose a reason for hiding this comment

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

That can only be changed in module/userconfig though.

The idea why it isn't inside SettingChanged is so that EH doesn't get installed at all, as if showPlayerNamesForce is enabled clients can't set it anyways.

What would you recommend for lines 27-29?

EDIT: 6774739 would solve it, I'd just throw entire thing under there.

Copy link
Member Author

Choose a reason for hiding this comment

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

@esteldunedain is working on the init thingy, was decided to leave as is for now.

@jonpas
Copy link
Member Author

jonpas commented May 18, 2015

Is there anything else I should be doing on this?

@nicolasbadano
Copy link
Contributor

Nah, hold on until that settings pr is merged.

@jonpas
Copy link
Member Author

jonpas commented May 19, 2015

Sounds good.

@jonpas
Copy link
Member Author

jonpas commented May 27, 2015

@esteldunedain What's up with settings PR in relation to this? Been over a week now.

@nicolasbadano
Copy link
Contributor

@jonpas, I just merged it. Sorry, I've been Witchering the whole week.

@jonpas
Copy link
Member Author

jonpas commented May 27, 2015

Thanks, happy Witchering!

Conflicts:
	addons/nametags/ACE_Settings.hpp
	addons/nametags/XEH_postInit.sqf
	addons/nametags/functions/fnc_onDraw3d.sqf
	addons/nametags/stringtable.xml
@jonpas
Copy link
Member Author

jonpas commented May 28, 2015

@esteldunedain This should be good to go. Tested, works well with loading up disabled/enabled and all that.

@jonpas
Copy link
Member Author

jonpas commented Jun 2, 2015

Any further info on approval of this?

nicolasbadano added a commit that referenced this pull request Jun 2, 2015
Disablable NameTags Module option
@nicolasbadano nicolasbadano merged commit 0022548 into acemod:master Jun 2, 2015
@nicolasbadano
Copy link
Contributor

Sorry, I had forgotten. My Witchering was interrupted by a bad flu and a fat pile of work, so I'm playing catchup atm. Please give it a quick test on master to ensure everything is still working as it should.

Thank you very much

@jonpas jonpas deleted the nametagsToggle branch June 2, 2015 20:24
@jonpas
Copy link
Member Author

jonpas commented Jun 2, 2015

Works good on my side, thanks for the merge and get healthy!

@nicolasbadano
Copy link
Contributor

Thanks man

@ViperMaul ViperMaul added this to the 3.1.0 milestone Jun 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants