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.

Sign up for GitHub

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

Standardize factory alt behavior #4177

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

Standardize factory alt behavior #4177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d23986
standardize factory alt behavior
hihoman23 Jan 15, 2025
db97d59
cleanup
hihoman23 Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions luaui/Widgets/cmd_factory_alt_behaviour.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
function widget:GetInfo()
return {
name = "Factory Alt Behavior",
desc = "Queueing with alt in a factory won't cancel the current command.",
author = "hihoman23",
date = "Jan 2025",
license = "GNU GPL, v2 or later",
layer = 0,
enabled = true
}
end

local spGiveOrderToUnit = Spring.GiveOrderToUnit
local spGetSelectedUnits = Spring.GetSelectedUnits
local spGetUnitDefID = Spring.GetUnitDefID
local spGetUnitWorkerTask = Spring.GetUnitWorkerTask

local CMD_INSERT = CMD.INSERT
local factoryBuildOpts = {}
for unitDefID, unitDef in pairs(UnitDefs) do
if unitDef.isFactory and unitDef.buildOptions then
local opts = {}
for _, opt in pairs(unitDef.buildOptions) do
opts[opt] = true
end
factoryBuildOpts[unitDefID] = opts
end
end

function widget:CommandNotify(cmdID, cmdParams, cmdOpts)
if cmdID < 0 and cmdOpts.alt and not cmdOpts.right then
local factoryExists = false
for _, unitID in ipairs(spGetSelectedUnits()) do
local unitDefID = spGetUnitDefID(unitID)
if factoryBuildOpts[unitDefID] and factoryBuildOpts[unitDefID][-cmdID] then
factoryExists = true
if not cmdOpts.internal then
cmdOpts.coded = cmdOpts.coded + CMD.OPT_INTERNAL -- prevent repeating command
Copy link
Collaborator

Choose a reason for hiding this comment

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

I slightly dislike using INTERNAL over ALT but I suppose that ship had already sailed.

Copy link
Member Author

Choose a reason for hiding this comment

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

What could be done to prevent this? I assume the factory will repeat commands, unless it has INTERNAL in the params.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can skip factories who are on repeat (since they already handle Alt correctly) and then for the remaining non-repeat ones you don't need to add internal since they won't repeat anyway.

end
local currentCmdID, targetID = spGetUnitWorkerTask(unitID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what happens if there is a unit in queue but the factory isn't actually building it (opening animation, area blocked, etc). There may be a minor behaviour difference compared to native engine insert. Sounds fine though.


-- insert command into wanted position
spGiveOrderToUnit(unitID, CMD_INSERT, {currentCmdID and 1 or 0, cmdID, cmdOpts.coded}, CMD.OPT_CTRL + CMD.OPT_ALT)
end
end
return factoryExists
Copy link
Collaborator

Choose a reason for hiding this comment

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

In theory this makes any fac in selection eat the entire order for all the other units. Sounds fine though.

Copy link
Member Author

Choose a reason for hiding this comment

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

It only does this if the unit is in the build options, so when it's relevant.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, but if you have a factory and a mobile unit who both have the same buildoption. Consider:

Beyond-All-Reason/units/Legion/Bots/T2 Bots/leginfestor.lua

Lines 43 to 44 in 9bb196c

buildoptions = {
[1] = "leginfestor"

Beyond-All-Reason/units/Legion/Labs/legalab.lua

Line 44 in 9bb196c

[9] = "leginfestor",

What happens if you have both selected and give a mobile build order while holding alt? Right now it looks like it's going to drop the mobile order but insert it in the factory instead.

Copy link
Member Author

@hihoman23 hihoman23 Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a weird one: in this case the factory already eats the command (in current master), even without alt (in both buildmenus). I would fix this in a future PR.

end
end
33 changes: 3 additions & 30 deletions luaui/Widgets/gui_gridmenu.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,17 +1043,6 @@ local function setPregameBlueprint(uDefID)
end
end

local function queueUnit(uDefID, opts)
local sel = spGetSelectedUnitsSorted()
for unitDefID, unitIds in pairs(sel) do
if units.isFactory[unitDefID] then
for _, uid in ipairs(unitIds) do
spGiveOrderToUnit(uid, -uDefID, {}, opts)
end
end
end
end

local function clearCategory()
setLabBuildMode(false)

Expand Down Expand Up @@ -1131,24 +1120,8 @@ local function gridmenuKeyHandler(_, _, args, _, isRepeat)
return false
end

local opts

if ctrl then
opts = { "right" }
Spring.PlaySoundFile(CONFIG.sound_queue_rem, 0.75, "ui")
else
opts = { "left" }
Spring.PlaySoundFile(CONFIG.sound_queue_add, 0.75, "ui")
end

if alt then
table.insert(opts, "alt")
end
if shift then
table.insert(opts, "shift")
end

queueUnit(uDefID, opts)
-- using SetActiveCommand has the advantage of CommandNotify being called
Spring.SetActiveCommand(spGetCmdDescIndex(-uDefID), ctrl and 3 or 1, not ctrl, ctrl, alt, false, meta, shift)

return true
end
Expand Down Expand Up @@ -2601,7 +2574,7 @@ function widget:UnitCmdDone(unitID, unitDefID, unitTeam, cmdID, cmdParams, optio
-- If factory is in repeat, queue does not change, except if it is alt-queued
local factoryRepeat = select(4, Spring.GetUnitStates(unitID, false, true))

if factoryRepeat and not options.alt then
if factoryRepeat and not options.internal then
return
end

Expand Down