-
Notifications
You must be signed in to change notification settings - Fork 299
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
feat(cmd_blueprint): add blueprint_select action #3881
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -267,7 +267,7 @@ local function setSelectedBlueprintIndex(index) | |
WG["api_blueprint"].setActiveBlueprint(nil) | ||
end | ||
|
||
if blueprintPlacementActive and index ~= nil and index > 0 then | ||
if index ~= nil and index > 0 then | ||
Spring.Echo("[Blueprint] selected blueprint #" .. selectedBlueprintIndex) | ||
end | ||
end | ||
|
@@ -864,6 +864,45 @@ local function handleSpacingAction(_, _, args) | |
end | ||
end | ||
|
||
local function handleBlueprintSelectAction(cmd, optLine, optWords, data, isRepeat, release, actions) | ||
local targetIndex = tonumber(optLine) | ||
|
||
local function selectIndex(index) | ||
setSelectedBlueprintIndex(index) | ||
Spring.SetActiveCommand( | ||
Spring.GetCmdDescIndex(CMD_BLUEPRINT_PLACE), | ||
1, | ||
true, | ||
false, | ||
false, | ||
false, | ||
false, | ||
false | ||
) | ||
end | ||
|
||
if targetIndex ~= nil then | ||
if targetIndex <= #blueprints then | ||
selectIndex(targetIndex) | ||
return true | ||
else | ||
return false | ||
end | ||
end | ||
|
||
local targetName = optLine | ||
if targetName ~= nil then | ||
for blueprintIndex, blueprint in ipairs(blueprints) do | ||
if blueprint.name == targetName then | ||
selectIndex(blueprintIndex) | ||
return true | ||
end | ||
end | ||
Comment on lines
+895
to
+900
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it make more sense to compile a map of blueprint name -> blueprint ID, so they can be selected directly by name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would probably be a bit faster, it would just need a bit more code, and performance shouldn't be an issue here. I'm ambivalent which way it's done, so I don't mind changing it. |
||
|
||
return false | ||
end | ||
end | ||
|
||
function widget:MousePress(x, y, button) | ||
if button ~= 1 or not blueprintPlacementActive or not getSelectedBlueprint() then | ||
return | ||
|
@@ -1145,6 +1184,7 @@ function widget:Initialize() | |
widgetHandler.actionHandler:AddAction(self, "blueprint_next", handleBlueprintNextAction, nil, "p") | ||
widgetHandler.actionHandler:AddAction(self, "blueprint_prev", handleBlueprintPrevAction, nil, "p") | ||
widgetHandler.actionHandler:AddAction(self, "blueprint_delete", handleBlueprintDeleteAction, nil, "p") | ||
widgetHandler.actionHandler:AddAction(self, "blueprint_select", handleBlueprintSelectAction, nil, "p") | ||
widgetHandler.actionHandler:AddAction(self, "buildfacing", handleFacingAction, nil, "p") | ||
widgetHandler.actionHandler:AddAction(self, "buildspacing", handleSpacingAction, nil, "p") | ||
|
||
|
@@ -1171,6 +1211,7 @@ function widget:Shutdown() | |
widgetHandler.actionHandler:RemoveAction(self, "blueprint_next", "p") | ||
widgetHandler.actionHandler:RemoveAction(self, "blueprint_prev", "p") | ||
widgetHandler.actionHandler:RemoveAction(self, "blueprint_delete", "p") | ||
widgetHandler.actionHandler:RemoveAction(self, "blueprint_select", "p") | ||
widgetHandler.actionHandler:RemoveAction(self, "buildfacing", "p") | ||
widgetHandler.actionHandler:RemoveAction(self, "buildspacing", "p") | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this echo needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case where blueprint placement is active, it's a bit redundant. If it's not (selecting while inactive is a new case this PR introduces), then it's the only indication that something happened. I could imagine removing a lot of the
Spring.Echo
s; I do feel like actions should give some feedback to the player though. I'm not sure what the best way to do that is.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make it not echo at least when the blueprint gui is open. It's a good compromise imo, since some feedback when nothing visible is seen is good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep in mind echos are for dev work, not for anything player-facing.