Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

View Restriction - Add setting to preserve view for vehicle types #107

Merged
merged 2 commits into from
Apr 20, 2018

Conversation

Dystopian
Copy link
Contributor

When merged this pull request will:

  • title.

This feature switches view on vehicle enter/exit to last used in this vehicle type/on foot. Supported types are the same as for Selective mode: LandVehicle, Air, Ship.
This setting will work only if View Restriction mode is Disabled.

Please higher attention to english strings.

To discuss:

  • preserve view for vehicle type + seat type (driver, cargo, turret)?

jonpas
jonpas previously requested changes Feb 9, 2018
QGVAR(preserveView),
"CHECKBOX",
[LSTRING(SettingPreserveViewName), LSTRING(SettingPreserveViewDesc)],
"ACEX " + localize LSTRING(ModuleDisplayName),
Copy link
Member

Choose a reason for hiding this comment

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

Use format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why? It's slower and not more readable.

Copy link
Member

Choose a reason for hiding this comment

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

You're right, for 2 strings it's slower. But such a tiny bit that I think just using format is better overall.

Copy link
Member

@jonpas jonpas Feb 11, 2018

Choose a reason for hiding this comment

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

In case of changes in the future, it's easier to add another, without risking worse performance loss. But this is so minimal in this case no one cares.

Actually why not just have "ACEX" as part of string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually why not just have "ACEX" as part of string?

It's already used in editor module name.

#include "script_component.hpp"

params ["_enabled"];
if (!_enabled || {GVAR(mode) > 0}) exitWith {
Copy link
Member

Choose a reason for hiding this comment

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

Newline above.

@jonpas jonpas added this to the 3.4.0 milestone Feb 9, 2018
@jonpas jonpas dismissed their stale review February 18, 2018 01:34

Requested changes addressed.

@jonpas
Copy link
Member

jonpas commented Feb 18, 2018

This setting will work only if View Restriction mode is Disabled.

Why exactly is that restriction? Could this not be solved by a simple check of what View Restriction mode is set to and then ignoring the view perservation?

@Dystopian
Copy link
Contributor Author

I just don't need it and don't want to implement those numerous ifs. No other reasons.

@jonpas
Copy link
Member

jonpas commented Apr 14, 2018

Good from my side, can be improved later if someone wants the functionality I mentioned above. Other ACE Settings also need to be moved to CBA Settings.

Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

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

Just two tiny things @Dystopian, small performance gain.


private _vehicle = vehicle _player;
private _vehicleClass = {if (_vehicle isKindOf _x) exitWith {_x}} forEach ["CAManBase", "LandVehicle", "Air", "Ship", "All"];
private _varName = QGVAR(preserveView) + _vehicleClass;
Copy link
Member

Choose a reason for hiding this comment

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

Use format [QGVAR(preserveView%1), _vehicleClass].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's slower not faster

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure? Last I remember format was faster. But that could be just on more concatenations or longer strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sure. It's faster for more than 2 strings. You said that too above #107 (comment) 😆

Copy link
Member

Choose a reason for hiding this comment

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

🤕

if !([_cameraView, cameraOn] call FUNC(canChangeCamera)) exitWith {};

private _vehicleClass = {if (_vehicle isKindOf _x) exitWith {_x}} forEach ["CAManBase", "LandVehicle", "Air", "Ship", "All"];
private _savedView = profileNamespace getVariable (QGVAR(preserveView) + _vehicleClass);
Copy link
Member

Choose a reason for hiding this comment

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

Use format [QGVAR(preserveView%1), _vehicleClass].

@jonpas jonpas merged commit ac9e281 into acemod:master Apr 20, 2018
@Dystopian Dystopian deleted the preserve_view branch April 20, 2018 23:56
@PabstMirror PabstMirror changed the title View Restriction: Add setting to preserve view for vehicle types View Restriction - Add setting to preserve view for vehicle types Aug 7, 2018
@PabstMirror PabstMirror modified the milestones: 3.4.0, 3.3.1 Aug 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants