-
-
Notifications
You must be signed in to change notification settings - Fork 155
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
Set unit names according to their faction #2804
Set unit names according to their faction #2804
Conversation
Hey, cheers for the PR. |
Names are taken from Normally, these names are referenced via Mods can declare their own name lists or use vanilla lists, for example all US-based factions use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you unmute the Civs?
Your Whole PR talks about Setting names and changing a few Functions, while you are silently unmuting Civs.
@Lazejun since civilian factions don't have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"this can be fixed later."
Well yes, if you add a Placeholder.
A3-Antistasi/A3A/addons/core/Templates/Templates/FactionDefaults/EnemyDefaults.sqf
Line 14 in 355b567
["attributesVehicles", []] call _fnc_saveToTemplate; |
"GreekMen" call _fnc_saveNames;
else you are gonna get for each faction thats not done yet:
12:57:30 Error in expression <tity getOrDefault ["lastName", ""];
if (_firstName != "" || _lastName != "") the>
12:57:30 Error position: <_firstName != "" || _lastName != "") the>
12:57:30 Error Undefined variable in expression: _firstname
12:57:30 File \x\A3A\addons\core\functions\Utility\fn_setIdentityLocal.sqf [A3A_fnc_setIdentityLocal]..., line 29
12:57:30 ➥ Context: [] L29 (\x\A3A\addons\core\functions\Utility\fn_setIdentityLocal.sqf [A3A_fnc_setIdentityLocal])
set default voices, faces and names for all factions, so the code may assume they are always present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and moving the identity-selection code to createUnit is an improvement. My one caveat is that you're passing more data over the network unnecessarily with the hashmap-parameters. It's probably negligible compared to other costs of creating units (esp. with our template system) but Arma's so inefficient at passing SQF data over the network that it makes me nervous.
On balance I think merge as-is and maybe optimize the parameter passing later if I get the urge. Should probably have been in the 3.3 test but that caught me a bit cold.
Co-authored-by: Jouni Järvinen <[email protected]>
Probably needs updating for the new templates (RHS chdkz, various SPE stuff, did I miss anything?). If you don't have SPE then I'll deal with it. |
New templates need to be adapted as well. |
Sorry for delay, don't have much time for ARMA right now. I never intended to configure all factions in a single PR, I don't even have most of paid DLCs. So I made this configuration optional. Unconfigured factions will work as before and can be configured later. If this is the only problem then I suggest to merge to unstable and test with many players. We can make small PRs for other factions later |
Yeah checked, it uses the template defaults to set greek voice/face/name, so it shouldn't break anything badly on new templates and should be good for merge. |
What type of PR is this.
What have you changed and why?
All units used to have Greek names regardless of their faction. Now unit names depend on their faction. NATO units have Western names, CSAT will have Chinese/Iranian names etc. Names were customized for vanilla, RHS and CUP factions, other faction will still use Greek names, this can be fixed later.
A3A_fnc_createUnit
now always sets the unit's identity to ensure that each unit has his identity configured. There was a bug when faction identity did not apply to rebel garrisons and maybe in some other cases.Please verify the following and ensure all checks are completed.
Is further testing or are further changes required?
How can the changes be tested?
Steps:
Find a field, look to the north and run