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

Nametags - Add Nation Ranks #7094

Merged
merged 8 commits into from
Oct 9, 2019
Merged

Nametags - Add Nation Ranks #7094

merged 8 commits into from
Oct 9, 2019

Conversation

Neciota
Copy link
Contributor

@Neciota Neciota commented Jul 8, 2019

When merged this pull request will:

  • Adds German, Spanish, British, and French rank icons for ace_nametags.
  • Adds config entries on existing A3 factions that can determine the ranks displayed in game.
  • Adds postInit code that loops through all factions, checks if it has rank config entries defined, and if so sets those for the faction.

This addition would allow mod makers to set custom ranks on their faction, so that British units are walking around with British rank insignia, Spanish with Spanish rank insignia, etc.
Letting mod makers define their own path allows them to add in custom rank insignia for countries that aren't defined yet.

Adds German, British, Spanish, French ranks. Also adds postInit code
that loops through all factions and checks if ranks have been set there
in there for easy, permanent customistability.
@Neciota
Copy link
Contributor Author

Neciota commented Jul 8, 2019

@commy2
Copy link
Contributor

commy2 commented Jul 8, 2019

  • Mod makers can already set custom icons by preInit script. I've been doing this in BWA3 already. This makes config entry support superfluous.

https://git.koffeinflummi.de/bwmod-team/bwa3-public/blob/master/bwa3_common/XEH_postInit.sqf#L8-21

  • ace_nametags_private -> GVAR(private)
  • "ace_nametags_private" -> QGVAR(private)
  • "\z\ace\addons\nametags\UI\icons_russia\private_gs.paa" -> QPATHTOF(UI\icons_russia\private_gs.paa)

Changes faction config entry to array with macros and changes the
postInit code to commy2's improved version.
@Neciota Neciota changed the title Adds icon and postInit Nation Ranks Jul 8, 2019
@commy2 commy2 self-assigned this Jul 8, 2019
Neciota and others added 2 commits July 9, 2019 15:34
Co-Authored-By: Dedmen Miller <dedmen@users.noreply.github.com>
Also removed white space.
Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

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

The two missing newlines @eof and this can be merged.

Neciota and others added 2 commits July 13, 2019 12:45
Co-Authored-By: commy2 <commy-2@gmx.de>
Co-Authored-By: commy2 <commy-2@gmx.de>
@commy2 commy2 added the kind/enhancement Release Notes: **IMPROVED:** label Jul 13, 2019
@commy2 commy2 added this to the 3.13.0 milestone Jul 13, 2019
@@ -45,3 +45,12 @@ GVAR(showNamesTime) = -10;

// civilians don't use military ranks
["CIV_F", ["","","","","","",""]] call FUNC(setFactionRankIcons);

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a way to disable, don't think it needs a setting.
We tend to use base vanilla units and just change the uniforms, so this doesn't really make sense for us, and would probably just be confusing
Does CSAT = russian and greek = german make sense?

Suggested change
if (missionNamespace getVariable [QGVAR(skipCustomRankIcons), false]) exitWith {};

Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you just overwrite the civs with whatever icons you want instead by using the same function?

Copy link
Contributor

Choose a reason for hiding this comment

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

civs are fine, I just want to skip the for loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Russian ranks and Iranian ranks are somewhat similar, the Chinese stand out somewhat in that they used downward point chevrons mostly. I don't know exactly how to reconcile in-game lore and geopolitics with the ranks, but if NATO gets all the American ranks, then maybe CSAT should get the Chinese ones? It's a little problematic that these nationally diverse alliances (CSAT/NATO) are one faction in game.

As for the AAF getting the German ranks; the AAF is mostly NATO-built force, and they have a bunch of equipment from all over NATO, so maybe they should just get NATO ranks.

When it comes to the new ranks, I'm not fussed as to who gets what. Just tell me what you need. If need be I'll happily add in more ranks for different nations and/or create some separate ranks for the AAF, FIA. The incoming Livonians should probably use the NATO ranks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Make some ranks for the NVA. :)
They'd be used in GM, and they are simple.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a getVar setting for this,
along with a check to make sure it doesn't overwrite existing entries
e.g. mission init.sq called setFactionRankIcons first

@PabstMirror PabstMirror changed the title Nation Ranks Nametags - Add Nation Ranks Aug 28, 2019
@commy2
Copy link
Contributor

commy2 commented Oct 9, 2019

Can't merge, because:

Some checks haven’t completed yet

@PabstMirror PabstMirror merged commit 6ca4b20 into acemod:master Oct 9, 2019
@jonpas
Copy link
Member

jonpas commented Oct 9, 2019

Commy, you should read #general on Slack more often. ;)

@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.13.0-temp2 Dec 30, 2019
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.

6 participants