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

Allow Hotbuild to redirect orders to exfac units like the buttons do #5656

Merged

Conversation

gordenwunderlich
Copy link
Contributor

Forged.Alliance.2023-11-10.16-09-34.mp4

should work for both normal and alternate hotbuild (templates not yet)

Needs more testing since I just quickly copied the code from @clyfordv but I wanted to at least put this out there.

@Garanas Garanas added the feature: mobile factories related to mobile factory functionality label Nov 10, 2023
@Garanas Garanas added this to the Development iteration IV milestone Nov 10, 2023
@Garanas
Copy link
Member

Garanas commented Dec 19, 2023

@4z0t can you take the time to review this? I generally do not use hotkeys

@Garanas Garanas added the feature: hotkeys related to miscellaneous hotkeys label Dec 19, 2023
Copy link
Member

@4z0t 4z0t left a comment

Choose a reason for hiding this comment

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

Follow DRY

lua/keymap/hotbuild.lua Show resolved Hide resolved
lua/keymap/hotbuild.lua Outdated Show resolved Hide resolved
lua/keymap/hotbuild.lua Outdated Show resolved Hide resolved
@gordenwunderlich
Copy link
Contributor Author

@4z0t
I moved the filter code to a different function, that also fixes the issue of declaring the local twice.
Should I put the part with if exFacUnits then into a separate function as well? It feels kinda weird to hide the simple if behind another function call.

lua/keymap/hotbuild.lua Outdated Show resolved Hide resolved
lua/keymap/hotbuild.lua Show resolved Hide resolved
@Garanas
Copy link
Member

Garanas commented Jan 8, 2024

There's no need to write a separate function for that if statement. If you can apply the Pascal convention to the name of the function today then we can include it with the hotfix

@gordenwunderlich
Copy link
Contributor Author

I changed the name of the new function, though all other functions in that file are still lowerCamelCase

@Basilisk3
Copy link
Collaborator

Tested and works. The only thing that does appear to be missing, is that the automatic enabling of repeat build does not work.

To remedy this, you can add categories.EXTERNALFACTORY and/or categories.EXTERNALFACTORYUNIT to line 684 in gamemain.lua. Though this will not solve this problem either, because the code requires you to select the factory for it to change it to repeat build. So if you never select the external factory, it will not have repeat build enabled automatically.

@MrRowey MrRowey marked this pull request as draft May 30, 2024 08:42
@Garanas Garanas changed the base branch from deploy/fafdevelop to develop June 1, 2024 19:36
@MrRowey
Copy link
Member

MrRowey commented Jun 12, 2024

@gordenwunderlich can you look at fixing what Basalisk mentions?

@Garanas
Copy link
Member

Garanas commented Jun 18, 2024

@MrRowey that can be fixed separately from this pull request.

@Garanas Garanas marked this pull request as ready for review June 18, 2024 14:49
@Garanas Garanas merged commit c20eac9 into FAForever:develop Jun 18, 2024
3 checks passed
Garanas added a commit that referenced this pull request Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: hotkeys related to miscellaneous hotkeys feature: mobile factories related to mobile factory functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants