Skip to content

Fixing Cyclops futily slow beaming a stinger by instead making it inch forwards #4698

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

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

Fixing Cyclops futily slow beaming a stinger by instead making it inch forwards #4698

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51c0004
Fixing Cyclops futily slow beaming a stinger by instead making it inc…
danfireman May 25, 2022
d4b7491
Addressing some PR feedback
danfireman Jul 24, 2022
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
68 changes: 68 additions & 0 deletions LuaRules/Gadgets/unit_cyclops_range_fix.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
function gadget:GetInfo()
return {
name = "Cyclops attack move fixing",
desc = "Makes Cyclops move closer when it can't hit with canon",
author = "dyth68",
date = "2022-05-12",
license = "Public Domain",
layer = 83,
enabled = true
}
end

local cyclopsi = {}
local cyclopsDefID = UnitDefNames["tankheavyassault"].id
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this would be a customParam so that people could make similar units use the same behaviour, or so that modders could easily remove this behaviour if they're using cyclops for a different purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched

local spGetUnitWeaponTestRange = Spring.GetUnitWeaponTestRange
local spGetUnitWeaponTarget = Spring.GetUnitWeaponTarget
local spGiveOrderToUnit = Spring.GiveOrderToUnit
local spGetUnitPosition = Spring.GetUnitPosition
local spGetTeamUnitsByDefs = Spring.GetTeamUnitsByDefs
local Echo = Spring.Echo
Copy link
Contributor

Choose a reason for hiding this comment

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

This should at least be spEcho, although in practice I never localise Spring.Echo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

local spGetUnitCommands = Spring.GetUnitCommands


function gadget:UnitCreated(unitID, unitDefID, teamId)
if unitDefID == cyclopsDefID then
Copy link
Contributor

Choose a reason for hiding this comment

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

See customparam comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

cyclopsi[unitID] = true
end
end


function gadget:UnitDestroyed(unitID, unitDefID, teamId)
if unitDefID == cyclopsDefID then
cyclopsi[unitID] = nil
end
end

function gadget:UnitGiven(unitID, unitDefID, newTeamID, teamID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unit given is redundant if the behaviour is completely team agnostic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

if unitDefID == cyclopsDefID then
cyclopsi[unitID] = true
end
end

function gadget:Initialize()
Copy link
Contributor

Choose a reason for hiding this comment

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

I like to put Init at the bottom of the gadget, with create/destroy above, with more frequent logic and callins above that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved

for _,teamID in ipairs(Spring.GetTeamList()) do
for _, unitID in ipairs(spGetTeamUnitsByDefs(teamID, cyclopsDefID)) do
cyclopsi[unitID] = true
end
end
end

function gadget:GameFrame(frame)
for cyclopsId, _ in pairs(cyclopsi) do
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally pairs would not be used. IterableMap with ApplySelf fits. It would even cleanly implement my next suggestion, which is that cyclops probably doesn't need to be checked every frame, and that we may as well stagger the checks over frames.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"IterableMap with ApplySelf " <- no idea what this is

I'd say you want to check the Cyclops often enough to make the inching forwards fairly smooth, so probably not less than once every 3 frames. Will wait to know more about this "ApplySelf" business before making a change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Grep iterableMap.

local _, _, canontargetID = spGetUnitWeaponTarget (cyclopsId, 1)
local targetType, _, slowBeamTargetID = spGetUnitWeaponTarget (cyclopsId, 2)
if (targetType == 1) and (canontargetID == nil) then
--Echo("Retargetting Cyclops")
local x, y, z = spGetUnitPosition(cyclopsId)
local tx, ty, tz = spGetUnitPosition(slowBeamTargetID)
local nx, nz = x * 0.95 + tx * 0.05, z * 0.95 + tz * 0.05
local ny = math.max(0, Spring.GetGroundHeight(nx, nz))
local cmd1 = spGetUnitCommands(cyclopsId, 1)[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

The unit might not have any commands, which would cause a crash. Also when getting the head of the queue you should use the callout that returns a non-table bunch of arguments that correspond to the first command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check for not having commands.

Not idea what you're referring to by the second sentence. Can't see anything relevant except GetUnitCommands at https://springrts.com/wiki/Lua_SyncedRead#CommandQueues

Copy link
Member

Choose a reason for hiding this comment

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

Spring.GetUnitCurrentCommand, search the zk repo for examples (it's not on wiki)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Errors when I try and use this.

if cmd1 and cmd1.options.internal then
spGiveOrderToUnit(cyclopsId, CMD.REMOVE, {1, cmd1.tag}, {"ctrl"})
end
spGiveOrderToUnit(cyclopsId, CMD.INSERT, {0, CMD.MOVE, CMD.OPT_INTERNAL, nx, ny, nz}, CMD.OPT_ALT)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are commands being removed and move commands inserted? Firstly, this should probably be CMD_RAW_MOVE. But secondly, does this need to be a command at all? It might be neater to implement this behaviour using a short move goal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Won't the internal check fail for attack commands the player issued? This sort of comment is a poor substitute for testing though, which I have not found the time to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent is to remove any existing inch-forwards command (otherwise you get a whole bunch of them and they mess things up).

Using CMD_RAW_MOVE seems to result in the Cyclops not inching.

No idea what a short move goal is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Move goals are direct orders for units to move somewhere. They sit below the command system. They can be neater for unit AI because they don't mess with commands at all.

Copy link
Contributor Author

@danfireman danfireman Sep 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That's not very helpful to me actually implementing it as I have no idea what you're talking about.

Copy link
Member

Choose a reason for hiding this comment

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

Spring.SetUnitMoveGoal(unitID, x, y, z, radius) makes the unit move towards given point (and be satisfied once in given radius)

end
end