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

Add Hotkeys for the unit and building commands #216

Draft
wants to merge 37 commits into
base: develop
Choose a base branch
from

Conversation

Rampoina
Copy link
Contributor

@Rampoina Rampoina commented Jul 18, 2022

This is a proposal to add hotkeys for every command.
This adds 10 configurable keys that map to the commands as displayed in the GUI.
It's meant to be used in a grid layout such as QWER ASDF ZXCV to make the locations more intuitive. (although it doesn't have to, users can set each hotkey wherever they want to)

Fixes #212

@andy5995
Copy link
Collaborator

@titiger would you like to test this with me and @Rampoina soon? Is this something you'd consider merging?

@Jammyjamjamman Jammyjamjamman self-requested a review July 19, 2022 21:55
@andy5995
Copy link
Collaborator

@Jammyjamjamman The controls are listed here in the README: https://raw.githubusercontent.com/MegaGlest/megaglest-source/master/docs/README.txt Is that generated from a script?

Copy link
Contributor

@Jammyjamjamman Jammyjamjamman left a comment

Choose a reason for hiding this comment

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

Looks good so far 👍 . @andy5995 and I gave it a test. I've listed changes that I think are required, based on our testing.

Copy link
Contributor Author

@Rampoina Rampoina left a comment

Choose a reason for hiding this comment

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

This commit adds the hotkey text on hover. It's going to need translations for the text "Hotkey".

@Jammyjamjamman Jammyjamjamman self-requested a review July 28, 2022 17:37
Copy link
Contributor

@Jammyjamjamman Jammyjamjamman left a comment

Choose a reason for hiding this comment

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

Looks good. I don't have the final say on merging this but the changes look good to me.

@pavanvo
Copy link
Contributor

pavanvo commented Aug 2, 2022

Yo @Rampoina , Good work!!! but wait up, tech engineer can't build Balista with hotkey. We need add at least 11 command.
And Summoner can't update to dragon, cuz we checking by numberCommands
at source/glest_game/gui/gui.cpp
line 460
if (i < numberCommands) {
mouseDownDisplayUnitSkills(i);
computeDisplay();
}
but some units like above have 3 commands in second line!

@Rampoina
Copy link
Contributor Author

Rampoina commented Aug 2, 2022

@pavanvo thanks for testing!
I've bumped the number of hotkeys to 12 (the full grid) and fixed the issue with the morphing units (numberCommands wasn't updated correctly again ... I don't know if I'm missing something here, it seems like a variable that should exist, but I wasn't able to find it)

@pavanvo
Copy link
Contributor

pavanvo commented Aug 2, 2022

@Rampoina you forget to commit gui.h
Now, with simple increment, it's working fine, but I'm trying to create another kind of check.

@Rampoina
Copy link
Contributor Author

Rampoina commented Aug 2, 2022

@pavanvo It's not only an increment, now it uses posDisplay which is updated inside the previous for, including the case for morphing units.
(I force pushed the missing gui.h in the same commit)

@andy5995
Copy link
Collaborator

andy5995 commented Aug 8, 2022

@Jammyjamjamman The controls are listed here in the README: https://raw.githubusercontent.com/MegaGlest/megaglest-source/master/docs/README.txt Is that generated from a script?

Seems like this hasn't been done yet. @Jammyjamjamman imo this should be done before this gets merged.

Copy link
Collaborator

@andy5995 andy5995 left a comment

Choose a reason for hiding this comment

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

Currently there are conflicts (as shown at the bottom).

pavanvo and others added 11 commits August 16, 2022 21:56
since we removed this hotkeys, we don't need this code any more
code blocking switching from attack to anothe command
…koutv3 (MegaGlest#250)

* workflows/cmake.yml:test on Ubuntu Jammy (22.04);migrate to checkoutv3

This should also fix MegaGlest#247

* maybe fix ssh link error on Ubuntu Jammy

gcc and clang build is failing on Jammy with the message:

'cannot find -lssh: No such file or directory'

Basically I added libcurl-openssl-dev to the deps

* remove libcurl4-gnutls-dev

Trying to correct:

 The following packages have unmet dependencies:
libcurl4-gnutls-dev : Conflicts: libcurl4-openssl-dev but
7.81.0-1ubuntu1.3 is to be installed
libcurl4-openssl-dev : Conflicts: libcurl4-gnutls-dev but
7.81.0-1ubuntu1.3 is to be installed
E: Unable to correct problems, you have held broken packages.
An error occurred while installing build dependencies.

* use cmake FindCURL module

* for OpenSSL, use include instead of find_package

* remove jammy, add gcc-10 and 11 test

* revert now-unrelated changes

* clean-up

* add VERBOSE flag to make

* Update .github/workflows/cmake.yml

* Update .github/workflows/cmake.yml

* use '-f' option from build script to force clang

* force dynamic libs with '-d'

fixes MegaGlest#251

* mk/linux/setupBuildDeps.sh:fix script so 22.04 is detected

*remove vlc deps (not required for the CI)
*remove ubuntu-18.04 from the build matrix, see
https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/

* revert removal of commented macos jobs

* run apt-get update and upgrade

* cleanup Prep snapshot section
@Jammyjamjamman
Copy link
Contributor

There's some mods that crash the game with this patch:

There might be more that don't work. I'm not sure what causes the crash. In e.g. prax the crash happens when loading "hexer".

@pavanvo
Copy link
Contributor

pavanvo commented Aug 28, 2022

There's some mods that crash the game with this patch:

* prax 0.5.9.7 (find in mod center)

* https://github.com/Robotkiller001/Megaglest-Improved-Mod

* https://github.com/Robotkiller001/Insects-World-Mod

There might be more that don't work. I'm not sure what causes the crash. In e.g. prax the crash happens when loading "hexer".

i am tested on linux and windows - everything works fine, I think problem in build
I found problem with "haxer"

@andy5995
Copy link
Collaborator

andy5995 commented Feb 26, 2025

I added a patch for these changes in #317 (There are some merge conflicts in this PR, so I fixed them locally first and then generated a patch using git diff)

@pavanvo
Copy link
Contributor

pavanvo commented Feb 26, 2025

Hi @andy5995 here most updated hotkeys branch
https://github.com/pavanvo/megaglest-source/tree/feat/hotkeys_rebase

@andy5995 andy5995 marked this pull request as draft February 26, 2025 11:43
@andy5995 andy5995 mentioned this pull request Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hotkeys for units, buildings and skills
4 participants