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

danfireman · 2022-05-25T22:00:12Z

TODO:

Further restrict when inching happens. Only do it when attack moving or attacking the slow beamed target
Be more careful about removing commands to specifically only remove when it's an inching move command
Verify performance drop when a hundred cyclops are spammed is under 1%

…h forwards

GoogleFrog · 2022-05-26T08:34:00Z

I feel like the engine is meant to have something that sets which weapon is for aiming. Failing that, how much range does slowbeam need to lose for the main gun to fire at reasonable elevation differences? If the fix is as simple as 20 less slowbeam range, then perhaps that's the one to go for.

danfireman · 2022-07-10T12:33:57Z

After a bit of looking I couldn't find any way to use the engine for this.

The slow beam needs to lose about 60 range before a modest slope or a hill 1.5 lotus high prevents it from just sitting there.
However, this is not a viable solution as losing any range at all means a Cyclops on attack move won't move into slow beam range against stingers or other static things like a afk paladin. It also can't catch and kill Dante any more and is MUCH easier for scalpels and domis to kite (two units tank really struggles against).

Shall I go ahead with my TODOs above on the assumption this will be merged?

GoogleFrog · 2022-07-11T13:31:05Z

I was planning to look prior to the next update. I'll have a look now and make some comments.

GoogleFrog · 2022-07-11T13:32:10Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+end
+
+local cyclopsi = {}
+local cyclopsDefID = UnitDefNames["tankheavyassault"].id


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.

GoogleFrog · 2022-07-11T13:32:36Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+local spGiveOrderToUnit        = Spring.GiveOrderToUnit
+local spGetUnitPosition        = Spring.GetUnitPosition
+local spGetTeamUnitsByDefs     = Spring.GetTeamUnitsByDefs
+local Echo                     = Spring.Echo


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

GoogleFrog · 2022-07-11T13:32:49Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+
+
+function gadget:UnitCreated(unitID, unitDefID, teamId)
+	if unitDefID == cyclopsDefID then


See customparam comment.

GoogleFrog · 2022-07-11T13:33:31Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+	end
+end
+
+function gadget:UnitGiven(unitID, unitDefID, newTeamID, teamID)


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

GoogleFrog · 2022-07-11T13:34:04Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+	end
+end
+
+function gadget:Initialize()


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

GoogleFrog · 2022-07-11T13:35:10Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+end
+
+function gadget:GameFrame(frame)
+	for cyclopsId, _ in pairs(cyclopsi) do


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.

"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.

Grep iterableMap.

GoogleFrog · 2022-07-11T13:37:08Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+			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]


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.

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

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

Errors when I try and use this.

GoogleFrog · 2022-07-11T13:40:30Z

LuaRules/Gadgets/unit_cyclops_range_fix.lua

+				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


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.

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.

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.

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.

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

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

GoogleFrog · 2022-11-05T04:34:36Z

I tried to move a Cyclops through Crab range and it gave itself a small move order and became stuck. The units in this picture are immobile.

This time I spawned a Cyclops in range and tried to move away. It got to the point where it could no longer fire, then got stuck on a small move order towards the Crab.

Moving a Cyclops away from a Nimbus is impossible.

GoogleFrog · 2022-11-05T04:38:42Z

I don't see any part of the code that would deal with these cases.

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

Grep for examples. Eg

Grep for examples again. Other gadgets use it. Experiment a bit.

Fixing Cyclops futily slow beaming a stinger by instead making it inc…

51c0004

…h forwards

GoogleFrog reviewed Jul 11, 2022

View reviewed changes

Addressing some PR feedback

d4b7491

GoogleFrog added the feedback required label Nov 5, 2022

sprunk marked this pull request as draft January 26, 2024 16:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

danfireman commented May 25, 2022 •

edited

Loading

GoogleFrog commented May 26, 2022

danfireman commented Jul 10, 2022 •

edited

Loading

GoogleFrog commented Jul 11, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 25, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

sprunk Jul 25, 2022

danfireman Sep 26, 2022

GoogleFrog Jul 11, 2022

GoogleFrog Jul 11, 2022

danfireman Jul 24, 2022

GoogleFrog Jul 25, 2022

danfireman Sep 26, 2022 •

edited

Loading

sprunk Sep 27, 2022

GoogleFrog commented Nov 5, 2022

GoogleFrog commented Nov 5, 2022 •

edited

Loading



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

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

Are you sure you want to change the base?

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

Conversation

danfireman commented May 25, 2022 • edited Loading

GoogleFrog commented May 26, 2022

danfireman commented Jul 10, 2022 • edited Loading

GoogleFrog commented Jul 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danfireman Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GoogleFrog commented Nov 5, 2022

GoogleFrog commented Nov 5, 2022 • edited Loading

danfireman commented May 25, 2022 •

edited

Loading

danfireman commented Jul 10, 2022 •

edited

Loading

danfireman Sep 26, 2022 •

edited

Loading

GoogleFrog commented Nov 5, 2022 •

edited

Loading