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

maptools - Improve behaviour upon first open (show under mouse position instead of bottom-left corner) #8848

Merged
merged 4 commits into from
Apr 6, 2022
Merged

maptools - Improve behaviour upon first open (show under mouse position instead of bottom-left corner) #8848

merged 4 commits into from
Apr 6, 2022

Conversation

b-mayr-1984
Copy link
Contributor

When merged this pull request will:

  • Show the Map Tools not in the bottom-left corner of the map when first opened, but right where your mouse is (as soon as mouse is moved slightly; might need further improvement here)

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
    - does not affect the documentation of Map Tools
  • Development Guidelines are read, understood and applied.
    - sorry no; such a small change does not justify reading through all of that :-/
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

Don't show the map tools at the bottom-left corner of the map when you first open it, but right where your mouse is.
@Drofseh
Copy link
Contributor

Drofseh commented Mar 17, 2022

With this PR, if you open the map tool, close it, move the map, then reopen the map tool, will it be at the previous position, or will it open at the new mouse position on the map?

@b-mayr-1984
Copy link
Contributor Author

With this PR, if you open the map tool, close it, move the map, then reopen the map tool, will it be at the previous position, or will it open at the new mouse position on the map?

It will open at the previous position, just as the map tool always did.
In order to preserve this behavior the flag GVAR(mapTool_isFirstShown) is used. After opening the map tool for the first time the flag is reset.

see

GVAR(mapTool_isFirstShown) = false; // we only need to do this once

@Drofseh
Copy link
Contributor

Drofseh commented Mar 17, 2022

Would you be willing to add a setting move it back to mouse position each time it's opened?

@PabstMirror
Copy link
Contributor

PabstMirror commented Mar 17, 2022

https://github.com/acemod/ACE3/blob/master/addons/maptools/CfgVehicles.hpp#L35 (lines 35 and 42)
could be changed to
statement = QUOTE(if (GVAR(mapTool_Shown)) == 0) then {GVAR(mapTool_isFirstShown) = true}; GVAR(mapTool_Shown) = 1;);

@PabstMirror PabstMirror added the kind/enhancement Release Notes: **IMPROVED:** label Mar 17, 2022
@PabstMirror PabstMirror added this to the Ongoing milestone Mar 17, 2022
@b-mayr-1984
Copy link
Contributor Author

b-mayr-1984 commented Mar 17, 2022

Wood you be willing to add a setting move it back to mouse position each time it's opened?

Well I would need to get acquainted with settings first. (I haven't done any modding yet. This is my first PR)

What kind of config method did you have in mind?

  • Ace settings like those for the medical system?
  • User controls like DUI does it?
  • something else?

The more I think of it, the more I think it's best to keep it simple. Meaning that I could rework it to always be centered when it is toggled to be shown.
Users that want to keep the position of the map tool can just keep it open.

What do you think of that? 🙂

@b-mayr-1984 b-mayr-1984 reopened this Mar 17, 2022
@b-mayr-1984
Copy link
Contributor Author

https://github.com/acemod/ACE3/blob/master/addons/maptools/CfgVehicles.hpp#L35 (lines 35 and 42) could be changed to statement = QUOTE(if (GVAR(mapTool_Shown)) == 0) then {GVAR(mapTool_isFirstShown) = true}; GVAR(mapTool_Shown) = 1;);

I will give it a go. Thanks 🙂

@Drofseh
Copy link
Contributor

Drofseh commented Mar 18, 2022

The more I think of it, the more I think it's best to keep it simple. Meaning that I could rework it to always be centered when it is toggled to be shown. Users that want to keep the position of the map tool can just keep it open.

What do you think of that? 🙂

Yeah that's fine by me, I'd like it to always move to mouse, so my suggestion it be a setting was just in case other people didn't want that.

Pabst's code should be good. Although I'd suggest renaming the variable, since now it won't just be on the first opening. Maybe something like GVAR(moveToMouse).


Well I would need to get acquainted with settings first. (I haven't done any modding yet. This is my first PR)

https://cbateam.github.io/CBA_A3/docs/files/settings/fnc_addSetting-sqf.html

If you did want to do a setting, instead of adding a bool in XEH_postInitClient.sqf you'd add one in initSettings.sqf, something like

[
    QGVAR(moveToMouse), "CHECKBOX",
    [LSTRING(moveToMouse_displayName), LSTRING(moveToMouse_description)],
    _category,
    true,
    0
] call CBA_fnc_addSetting;

strings in stringtable.xml

        <Key ID="STR_ACE_MapTools_moveToMouse_displayName">
            <English>Always open map tool to mouse position.</English>
        </Key>
        <Key ID="STR_ACE_MapTools_moveToMouse_description">
            <English>By default the map tool will only move to mouse position the first time it is opened, enable this to move it every time it's opened.</English>
        </Key>

Then you'd have a bool called GVAR(moveToMouse) you could use where needed, and the user could set it to true or false as they like

incorporated feedback from co-authors

Co-Authored-By: Drofseh <Drofseh@users.noreply.github.com>
Co-Authored-By: PabstMirror <pabstmirror@gmail.com>
@b-mayr-1984
Copy link
Contributor Author

If you did want to do a setting, instead of adding a bool in XEH_postInitClient.sqf you'd add one in initSettings.sqf, something like

[
    QGVAR(moveToMouse), "CHECKBOX",
    [LSTRING(moveToMouse_displayName), LSTRING(moveToMouse_description)],
    _category,
    true,
    0
] call CBA_fnc_addSetting;

strings in stringtable.xml

        <Key ID="STR_ACE_MapTools_moveToMouse_displayName">
            <English>Always open map tool to mouse position.</English>
        </Key>
        <Key ID="STR_ACE_MapTools_moveToMouse_description">
            <English>By default the map tool will only move to mouse position the first time it is opened, enable this to move it every time it's opened.</English>
        </Key>

Then you'd have a bool called GVAR(moveToMouse) you could use where needed, and the user could set it to true or false as they like

Thanks @Drofseh for the advice here. :-)
I started implementing it but then stopped because I believe this will most likely be code that is close to dead-code that barely has any relevance. I couldn't imagine many people using anything but the default (which is always move to mouse). So I can not justify this amount of code and the need for translating more setting strings. But I keep your instructions in mind for future reference. :-)

@PabstMirror PabstMirror modified the milestones: Ongoing, 3.14.2 Mar 19, 2022
@PabstMirror
Copy link
Contributor

One minor thing I noticed is the tool only shows after moving the mouse
because the logic is only in a MouseMoving EH
e.g. you do the show map tools action and nothing happens until you wiggle the mouse a little

We should just move the logic to fnc_updateMapToolMarkers around line 21

// open map tools in center of screen when toggled to be shown
if (GVAR(mapTool_moveToMouse)) then {
    private _mousePosition = _theMap ctrlMapScreenToWorld getMousePosition;
    GVAR(mapTool_pos) = _mousePosition;
    GVAR(mapTool_moveToMouse) = false;  // we only need to do this once	after opening the map tool
};

I can do this if it sounds ok

@b-mayr-1984
Copy link
Contributor Author

One minor thing I noticed is the tool only shows after moving the mouse because the logic is only in a MouseMoving EH e.g. you do the show map tools action and nothing happens until you wiggle the mouse a little

We should just move the logic to fnc_updateMapToolMarkers around line 21

I can do this if it sounds ok

Yes that would be awesome. I am kind of embarrassed that it is that simple to change it, because I really tried to find the proper place for the code in order to not need the mouse to be moved. But I guess I am just not familiar enough with SQF and the entire Arma and ACE API yet. 😊

I am more than happy with you doing it. 🙂
Let me know if you need anything else from my side.

@PabstMirror PabstMirror merged commit 020cf2d into acemod:master Apr 6, 2022
@b-mayr-1984 b-mayr-1984 deleted the first_open_of_mapTools_center_of_screen branch April 6, 2022 21:55
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.

3 participants