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

Introducing zeus interaction menu #1519

Merged
merged 9 commits into from
Jun 27, 2015
Merged

Introducing zeus interaction menu #1519

merged 9 commits into from
Jun 27, 2015

Conversation

kymckay
Copy link
Member

@kymckay kymckay commented Jun 5, 2015

Here you can find the related issue on my fork

@jaynus had the original idea to add a GM menu to the game after finding that constantly editing the attributes of things in zeus was slow (stance, formation, etc.).

Originally I implemented something like this. The benefits of this implementation were:

  • Would work from any camera view (zeus, regular game, spectator, etc.)
  • Was entirely separate from ace_interact_menu
  • Probably more that I'm forgetting

However, in testing I found that it was a little impractical to use for the following reasons:

  • Can't directly interact with anything other than objects (groups/waypoints)
  • Interaction is fiddly when objects are moving around (as they often are)
  • Due to the limitations imposed by the process of collecting objects to interact with you'd have to at least be semi-near to objects to be able to interact with them

This implementation still exists under the gm_menu branch of my fork. However I've since made a second implementation (this PR) on branch gm_revamp. Here are previews of the second version: 1, 2.

This version isn't so much a fully fledged GM menu, instead it's limited to the zeus interface and acts as an extension of regular interaction. The benefits of this implementation:

  • Simple code, zeus provides a bunch of what's needed already:
    • Accuracy - you can select exactly what you want to interact with wherever it is
    • Security - restricted access is built into zeus already
    • Functionality - already supports interaction with waypoints/groups/markers
  • Easy to use. It's rendered like a self interaction menu so there's no problem with moving objects
  • Batch interaction. You can interact with multiple groups/objects/waypoints/markers at once due to the selection system's flexibility. I find this to be an extremely fitting solution to the original problem.

The two main drawbacks are that it's limited to the zeus interface (I don't think this is a big deal) and that it's extending interact_menu instead of being separate. I imagine a separate version can also be made, it would just require some decisions to be made concerning how to implement it.

@thojkooi thojkooi added kind/enhancement Release Notes: **IMPROVED:** kind/feature Release Notes: **ADDED:** labels Jun 6, 2015
@thojkooi thojkooi added this to the 3.2.0 milestone Jun 6, 2015
@nicolasbadano
Copy link
Contributor

Congratz @SilentSpike, this looks very good. Extending the interact_menu instead of duplicating code is a good solution, given that this doesn't complicate the code all that much.

I'll review this in detail over the next few days.

One detail that we may want to correct is that the zeus interface disappears when the menu opens, making which objects are selected invisible. If we can manage to avoid that by e.g. opening a dialog it would be great.

The other thing is that the Zeus interaction should always use the cursor, as it really wouldn't make sense to select options by freelooking.

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

Line 37 of fnc_keyDown should satisfy the cursor requirement.

As for not closing the zeus interface, I actually showed this to moricky and he suggested that too. It's an easy change to apply (change line 50 of fnc_keyDown to use createDialog if !isNull curatorCamera).

However, with that change applied the selected objects are still no longer drawn, though the menu on the left will still show them selected.

Two things I also had in mind:

  • One thing I would like to find a solution for is noted in a comment on line 76 of XEH_clientInit. I'm not sure if CBA provides a way to enable only certain keybindings rather than all of them.
  • Also, it feels a lot nicer when fnc_keyDown is edited to stop the cursor from being centered, however I'm not entirely sure if that messes anything up elsewhere (as GVAR(cursorPos) is being initialized as [0.5,0.5,0] on line 55).

@nicolasbadano
Copy link
Contributor

As for not closing the zeus interface, I actually showed this to moricky and he suggested that too. It's an easy change to apply (change line 50 of fnc_keyDown to use createDialog if !isNull curatorCamera).

Yeah, I think we should do that. It will probably look less confusing

However, with that change applied the selected objects are still no longer drawn, though the menu on the left will still show them selected.

Ok, well, tough luck. I think it will have to do.

One thing I would like to find a solution for is noted in a comment on line 76 of XEH_clientInit. I'm not sure if CBA provides a way to enable only certain keybindings rather than all of them.

No, I don't think it does. However I think the current approach is fine, did you find any problem with it?

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

No immediate problem, allowing self interaction also makes some sense from zeus view. However it means all CBA keybindings will work there, which could be problematic for other addons if they don't account for it in their keybinding code.

@nicolasbadano
Copy link
Contributor

which could be problematic for other addons if they don't account for it in they keybinding code.

Good point. FTM I think will have to stick with this solution. Going forward I think we could propose to CBA that keybindings should have configurable "scopes", to define if they should work only on the main display or in other places as well.

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

I did notice on the CBA issue tracker: CBATeam/CBA_A3#33

Should keep an eye on it, I agree that this will have to do for now though.

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

Menu no longer minimizes the interface.

I was also wondering about the interaction configs themselves, should they be in interact_menu, interaction or even somewhere else?

@nicolasbadano
Copy link
Contributor

I was also wondering about the interaction configs themselves, should they be in interact_menu, interaction or even somewhere else?

Totally right. They should not be on the interact_menu, which only provides te menu framework, not the actions. I'd think something like ACE_Zeus, or even their own pbos. Particular pbos could maybe also add their own entries (e.g., ace_captives adding options to capture/uncapture soldiers)

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

Yup, that makes sense. I suppose it makes sense if ACE_zeus provides the actions that mimic existing zeus functionality then.

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

Though, perhaps what should be kept with the framework are the base branches (units, groups, waypoints and markers)?

Depends how we envision other addons to make their own entries.

@nicolasbadano
Copy link
Contributor

Though, perhaps what should be kept with the framework are the base branches (units, groups, waypoints and markers)?

Depends how we envision other addons to make their own entries.

The basic classes for regular interactions are on the ace_interaction pbo. Maybe that could be a better place to put the basic zeus action classes too, given that that pbo is required by most other pbos already

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

Yeah, I like that idea.

ace_interaction can have all the basic zeus functionality actions, which means other addons can also branch off of those main 4 branches if they wish.

The only problem I foresee is that there could be a lot of actions for this one menu. However we should probably just deal with that if and when it becomes an issue.

@nicolasbadano
Copy link
Contributor

The only problem I foresee is that there could be a lot of actions for this one menu. However we should probably just deal with that if and when it becomes an issue.

Sure, we can probably create some submenus as needed

@kymckay
Copy link
Member Author

kymckay commented Jun 6, 2015

Moved, also updated some of the strings to use new ones in stringtable.xml (as the vanilla ones had colons - and for some reason "careless" doesn't follow the same naming convention as the rest of the behaviours)

@kymckay
Copy link
Member Author

kymckay commented Jun 7, 2015

Interaction is prevented under certain conditions (unconscious, captive, etc.) which also prevent zeus interaction. I need to figure out if there's a trigger for this provided by interact_menu (which I can add an exception into for zeus) or if respective addons are just overriding interaction in their own code.

@nicolasbadano
Copy link
Contributor

I need to figure out if there's a trigger for this provided by interact_menu (which I can add an exception into for zeus)

Those checks are done here:
https://github.com/acemod/ACE3/blob/master/addons/interact_menu/XEH_clientInit.sqf#L34

https://github.com/acemod/ACE3/blob/master/addons/interact_menu/XEH_clientInit.sqf#L43

I think it would be safe to move those checks to fnc_keyDown.sqf and provide an exception if Zeus is the one interacting

@kymckay
Copy link
Member Author

kymckay commented Jun 7, 2015

Aha, thanks. I just finally came to understand how the whole condition/exception system works :)

@kymckay
Copy link
Member Author

kymckay commented Jun 7, 2015

Oh, while this is open, I noticed that in the keyUp function openedMenuType is set to -1 before the interactMenuClosed event is called (which passes openedMenuType as an argument).

Is this incorrect? I might as well fix it if so.

@nicolasbadano
Copy link
Contributor

Is this incorrect? I might as well fix it if so.

Yeah, it's an error, although I don't think we are using it anywhere atm. You may as well fix it while you are at it

@kymckay
Copy link
Member Author

kymckay commented Jun 12, 2015

Anything anyone thinks needs to be done here?

@kymckay
Copy link
Member Author

kymckay commented Jun 27, 2015

When do we wanna merge this? It's feature ready in my opinion.

nicolasbadano added a commit that referenced this pull request Jun 27, 2015
Introducing zeus interaction menu
@nicolasbadano nicolasbadano merged commit f52babe into acemod:master Jun 27, 2015
@nicolasbadano
Copy link
Contributor

Merged! Sorry, I forgot to do it previously

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:** kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants