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

Interact Menu - Add settings for self interaction menu reordering #8036

Merged
merged 7 commits into from
Jul 23, 2021

Conversation

Dystopian
Copy link
Contributor

@Dystopian Dystopian commented Dec 7, 2020

When merged this pull request will:

  • add More submenu to unit self interaction menu and settings to move any root menu items there;
  • add settings to move any submenu items to root.

I often use self interaction menu. But some items I use very often and some ones rarely. E.g. I put earplugs in each time I enter static/vehicle weapon and put them out on exit (to hear better). On the other hand I use Team management only once during mission and Create Zeus once in several missions. I don't want to disable Gestures and Check air temperature but they are better for me in More submenu. So I would like to move earplugs to root and hide others in More submenu.

All settings are disabled by default and require mission restart. To use them:
- Settings - ACE Interaction menu - ACE Interaction menu (Self) - Allow reordering;
- restart mission;
- Settings - ACE Interaction menu (Self) - More/Move to Root - check what you need;
- restart mission.

@jonpas jonpas added this to the 3.14.0 milestone Dec 13, 2020
@veteran29 veteran29 self-requested a review December 17, 2020 10:11
@jonpas
Copy link
Member

jonpas commented Apr 20, 2021

Interesting!

I am wondering if we really need the overall "Allow Reordering" setting, it seems to just add complexitiy. We could just generate all relevant settings on start, I don't see a real downside.

@veteran29 ?

@Dystopian
Copy link
Contributor Author

I am wondering if we really need the overall "Allow Reordering" setting, it seems to just add complexitiy. We could just generate all relevant settings on start, I don't see a real downside.

I had 2 reasons:

  1. Move to Root menu is quite big and is opened with noticeable lag. Some time ago @dedmen said in ACE Slack that he found reason of this for game settings (as I understood) and if it's fixed in future this point is out.
  2. this settings can be not very intuitive for newbies.

Personally I don't care - please you decide.

@jonpas
Copy link
Member

jonpas commented Apr 29, 2021

Maybe I wasn't clear enough or I don't know if I understood you. If I am reading this correctly, when user enables "Allow Reordering" setting and restarts the mission, user gets additional settings for additional actions "Move X to Root", "Move Y to Root", correct?

I am not sure what @dedmen's concerns were (can you enlighten us @dedmen?), but if it's not a few seconds of delay, then I am fine with there being many settings.

How many settings are there in total with all of ACE3 anyways?

@Dystopian
Copy link
Contributor Author

If I am reading this correctly, when user enables "Allow Reordering" setting and restarts the mission, user gets additional settings for additional actions "Move X to Root", "Move Y to Root", correct?

Additional setting categories Interaction menu (self) - More and Interaction menu (self) - Move to root appear.

I am not sure what @dedmen's concerns were (can you enlighten us @dedmen?), but if it's not a few seconds of delay, then I am fine with there being many settings.

How many settings are there in total with all of ACE3 anyways?

I have a delay about 1.2s on my PC when opening Move to root category. There are about 80 entries there.

@jonpas
Copy link
Member

jonpas commented Apr 29, 2021

Is it that slow simply because there are 80 settings in there? You don't do anything on the fly, everything happens on the newControllableObject event. So it's only CBA UI being slow with that amount of settings? (Not saying that it should be faster.)

What about splitting that large menu into multiple smaller menus to make it faster? Maybe Equipment, Interaction, ..., Other?

@Dystopian
Copy link
Contributor Author

Is it that slow simply because there are 80 settings in there? You don't do anything on the fly, everything happens on the newControllableObject event. So it's only CBA UI being slow with that amount of settings? (Not saying that it should be faster.)

Yes.

What about splitting that large menu into multiple smaller menus to make it faster? Maybe Equipment, Interaction, ..., Other?

There are many ACE categories in settings already. I don't think splitting this setting to submenu is cool. Also I think (at least from my experience) this setting is opened and used only once, so delay is not a real problem.

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.

I don't think the delay is a problem either. As long as it isn't too long to trigger ACRE/TFAR disconnect.

I say we get rid of the "Allow Reordering" setting. @PabstMirror @BaerMitUmlaut @mharis001 (and others) What's your take on this?

@mharis001
Copy link
Member

I agree with @jonpas . I think the "Allow reordering" setting is not needed; these additional settings should always be available.

I terms of performance, I didn't notice any delays other than the expected one when selecting the "Move to Root" settings category. Even that wasn't too bad - not longer than a second or so.

@dedmen
Copy link
Contributor

dedmen commented May 18, 2021

Some time ago @dedmen said in ACE Slack that he found reason of this for game settings (as I understood) and if it's fixed in future this point is out.

That was checkboxes in game settings menu making texture lookups each.
I recently greatly optimized texture lookup speed on profiling branch, that should mitigate that

@jonpas jonpas requested a review from mharis001 July 23, 2021 14:00
@jonpas jonpas modified the milestones: 3.14.1, 3.14.0 Jul 23, 2021
@jonpas jonpas merged commit 35290a1 into acemod:master Jul 23, 2021
@Dystopian Dystopian deleted the menu-reorder branch July 23, 2021 22:32
@jonpas jonpas added kind/feature Release Notes: **ADDED:** and removed kind/added feature labels Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants