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

Improvements to drone controls #1775

Merged
merged 8 commits into from Dec 30, 2016
Merged

Conversation

Aquanim
Copy link
Contributor

@Aquanim Aquanim commented Dec 26, 2016

  • Set Target on a carrier results in the same drone behaviour as an Attack order. (Set-target is overridden by Attack, and is not cancelled if a Move order is given to the carrier, as you would expect.)
  • Drones inherit their firestate from their carrier. (Probably most useful for a drone bomber.)
  • Added a button to recall drones to the mothership.
    -- EDIT: Recall no longer cancels orders given to the carrier; it just tells the drones to ignore them for a while.
    -- I am not a graphics designer; improvements to the recall icon are welcomed.
    -- Commanders get the recall drones button if and only if they have drones.
    -- When the carrier is given a recall order, the drones will hover around the carrier for ~10 seconds before returning to normal operation.
    -- An attack, fight, patrol or set-target order given to the carrier cancels the remainder of the drone recall time.

It would be a good idea for somebody else to test this before merge/stable, not only for bugs but also to see if this control scheme is intuitive to other people.

The carrier's target and any active or immediately queued fight/attack moves are cancelled.
The button icon is a placeholder; improvements are invited.
Commanders get the recall drones button if they have drones.
The drones will hover around the carrier for ~10 seconds before returning to normal operation.
@sprunk
Copy link
Member

sprunk commented Dec 26, 2016

As far as I can tell you're setting the recall_frame_start rules param to -RECALL_TIMEOUT as a form of removal, including on newly created units. At the same time you're still checking it for nil.

If you're guaranteeing the param to be non-nil, consider dropping the nil checks.
Since GoogleFrog would probably raeg at dropping nil checks, alternatively consider explicitly setting the param to nil when removing.

Also rearranged code a little.
Should be no player-side change.
@Aquanim
Copy link
Contributor Author

Aquanim commented Dec 27, 2016

The second option seemed more reasonable.

@@ -90,6 +107,8 @@ local function Drones_InitializeDynamicCarrier(unitID)
if drones then
carrierData[#carrierData + 1] = data
maxDronesOverride[#maxDronesOverride + 1] = drones
Spring.InsertUnitCmdDesc(unitID, recallDronesCmdDesc)
spSetUnitRulesParam(unitID,"recall_frame_start",nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is just initializing the unit (as the previous line suggests) then this line does nothing and should be removed. Setting the unit rules param to nil would only do something if it previously had a value.

function gadget:UnitCreated(unitID, unitDefID, unitTeam)
if (carrierDefs[unitDefID]) then
Spring.InsertUnitCmdDesc(unitID, recallDronesCmdDesc)
spSetUnitRulesParam(unitID,"recall_frame_start",nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely this is not required.

@GoogleFrog GoogleFrog merged commit 871bac9 into ZeroK-RTS:master Dec 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants