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

Feature: area marker visibility by player side #342

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Fusselwurm
Copy link

@Fusselwurm Fusselwurm commented Mar 27, 2020

Add a new feature to the area marker configuration.

By default, new area markers will be visible to all players as before:

20200327122830_1

But markers can be made invisible to some or all sides, like here - marker is only visible to blufor and civilian players:

20200327122305_1

Test cases:

  • visibility by side
    • Zeus Z creates marker
    • non-Zeus player A can see the marker
    • Z disables visibility for side A belongs to by clicking the side icon (in marker configuration dialog)
    • marker should disappear for A
    • Z can fiddle with other parameters of that markers without it re-appearing for A
    • Z can make the marker for A visible again by clicking their side icon (in marker configuration dialog)
  • more than one Zeus
    • Zeus Z creates marker and disables visbility
    • Zeus Y should continue to see the marker

@CreepPork CreepPork added the feature Adds a new feature label Mar 27, 2020
@CreepPork CreepPork added this to the Ongoing milestone Mar 27, 2020
@Fusselwurm
Copy link
Author

Fusselwurm commented Mar 29, 2020

I have a weird issue. The way I implemented this is by exchanging setMarkerAlpha (run by Zeus) with setMarkerAlphaLocal (run by all clients via cba global event) to hide the marker on certain clients

Works well, except for one thing:

  • given a marker that is visible to Zeus, and invisible to another client
  • when Zeus doubleclicks the marker (to open the configuration)
  • on the other client, the marker's alpha value mysteriously changes from 0 to either 1 (or to whatever the Zeus sees, not sure yet) ⚡️

From what I can see logging and looking into the profiler, neither the applyProperties nor the updateAlpha method are called, so... looks like our code is not to blame.

My suspicion: engine somehow re-broadcasts global marker when I touch it.

Tentative conclusion: having a global marker and then running *Local commands on it invites trouble.
edit: ah, there is even a warning in the biki

Will refactor to exclusively use local marker, see if that works better 😬

@mharis001
Copy link
Member

The updateAlpha event is currently only added on the server.

@Fusselwurm
Copy link
Author

Fusselwurm commented Mar 29, 2020

The updateAlpha event is currently only added on the server.

ah right, had forgotten to push the fix to that. 😓 the locality problem I described existed regardless.

I just refactored the whole thing to use local markers only, and tested with a buddy and it seems to work. 👍

things I still have to test :

  • with more than one zeus
  • dragging markers

@mharis001
Copy link
Member

It happens because onMouseButtonUp is called when the icon is double clicked, which calls setMarkerPos and it seems that any global marker commands will re-synchronize all marker properties.

@Fusselwurm
Copy link
Author

aaah. right. ok. - so are you okay with the "let's be safe and do local markers only" solution, or should I keep global markers and try and change the setMarkerPos to setMarkerPosLocal ? @mharis001

@mharis001
Copy link
Member

I think the simpler solution is probably better - if the global setMarkerPos is the only issue then that can be changed. However, if there are other problems, local markers would make sense then.

@Fusselwurm Fusselwurm force-pushed the feature_area-marker-visbility branch 3 times, most recently from 4c85862 to fea8d01 Compare April 3, 2020 20:07
@Fusselwurm Fusselwurm force-pushed the feature_area-marker-visbility branch from fea8d01 to d94d8e2 Compare April 3, 2020 20:08
@Fusselwurm Fusselwurm marked this pull request as ready for review April 3, 2020 20:08
addons/area_markers/functions/fnc_applyProperties.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_configure.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateAlpha.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateAlpha.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateAlpha.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateMarkerPos.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateMarkerPos.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateMarkerPos.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateMarkerPos.sqf Outdated Show resolved Hide resolved
addons/area_markers/functions/fnc_updateAlpha.sqf Outdated Show resolved Hide resolved
@CreepPork
Copy link
Member

Area marker documentation/images should be updated too.

@mharis001 Should it be handled by this PR or not?

@mharis001
Copy link
Member

Either works, we can do it later in another PR if that is easier.

@mharis001
Copy link
Member

This should be changed to use #362's common sides control for the UI handling. Currently, the icons are white when hovered and change side instantly when clicked.

@mharis001
Copy link
Member

Some things that need to be taken care of for this:

  • playerSide does not change if the player joins a group of a different side.
  • verify that it works as expected for JIP players.
  • consider showing all markers to curators when they are in the Zeus interface only, not always.
  • fix configure display positioning.

@@ -30,8 +30,17 @@ if (isServer) then {
[QGVAR(deleteIcon), _marker] call CBA_fnc_globalEvent;
};
}] call CBA_fnc_addEventHandler;

#define SIDES_ARRAY_HASH [[], [east, west, independent, civilian]] call CBA_fnc_hashCreate;
Copy link
Member

Choose a reason for hiding this comment

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

Use new 2.02 hash map

@mharis001 mharis001 modified the milestones: Ongoing, Backlog May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants