Sync "Drop pod raid at location"

[Tick-git]

Label: 1.6, fix

Additional Notes on the "Drop pod raid at location..." Debug Option:

This debug option was introduced with RimWorld 1.6.

While testing this the PR, I encountered consistent crashes when setting the raid points above approximately 750. This issue occurs even in an unmodded version of the game and has been confirmed by SirWilliams.

I haven’t yet found an existing bug report about this on the RimWorld Discord, though I didn’t search extensively. If no report surfaces, I plan to report the issue myself.

Reproduce mentioned crash:

• Rimworld 1.6
• No mods - only core
• Dev mode enabled
• Dev Quicktest
• Open Dev Tools
• Drop pod raid at location...
• Ancients(Ancients) [NO]
• 1000 pt
• 3 tiles

PR

This PR fixes #556 and synchronizes the final step in the "Drop pod raid at location..." debug option, which uses the Targeter.BeginTarget method. While this method doesn't appear to be used elsewhere in the debug system, it is invoked by other parts of the game. Essentially, it takes an Action and executes it when the user clicks on a target.

I implemented the fix in a similar fashion to the other 'DebugSource' enums.

The main challenge I faced was ensuring that my prefix runs only when Targeter.BeginTarget is invoked from within the DebugActionsIncidents.ExecuteDropPodRaidAtLocation method - or at the very least, from somewhere within the DebugActionsIncidents class. To achieve this, I used a bit of reflection to check in which class the action was created via ActionIsCreatedInDebugActionsIncidents (shown below). I'm not entirely sure if this is an acceptable approach.

Feedback on better alternatives is welcome.

private static bool ActionIsCreatedInDebugActionsIncidents(Action<LocalTargetInfo> action)
{
    return action?.Method.DeclaringType?.DeclaringType == typeof(DebugActionsIncidents);
}

Test

The fix was tested primarily on dev

• verified functionality on both host and client.
• Confirmed it works when both host and client are at the final step of the debug tree and perform the mouse click.
• The guard using reflection was tested by manually calling Targeter.BeginTarget via DebugSync.ReadCmd. I was not able to trigger this path naturally through regular gameplay in a reasonable amount of time.

[Tick-git]

To achieve this, I used a bit of reflection to check in which class the action was created via ActionIsCreatedInDebugActionsIncidents (shown below). I'm not entirely sure if this is an acceptable approach.

Feedback on better alternatives is welcome.

private static bool ActionIsCreatedInDebugActionsIncidents(Action<LocalTargetInfo> action)
{
    return action?.Method.DeclaringType?.DeclaringType == typeof(DebugActionsIncidents);
}

Ignore that for now - I might already have a better solution

[Tick-git]

Never mind - I was thinking the guard below might be sufficient, but after reviewing other patches for Targeter.BeginTargeting, I noticed that the patch Feedback.CancelBeginTargeting is also used in a synced command.

if (!Multiplayer.ExecutingCmds) return true;

The PR works as-is, but the current guard could be considered a bit hacky.

I'm open to feedback again :)

[notfood]

It could use an alternative, it's a very slow and ugly hack.

[Tick-git]

I've been thinking about it a bit more.

Idea A:

DebugSync.HandleCmd

public static bool executingListerAction;

public static void HandleCmd(ByteReader data)
{
    ...

    var state = Multiplayer.game.playerDebugState.GetOrAddNew(currentPlayer);

    ...
    
    Log.Message($"Debug tool {source} ({cursorX}, {cursorZ}) {currentHash} {path}");

    try
    {
        ...

        else if (source == DebugSource.Lister)
        {
            executingListerAction = true;                                      // =======> SET FLAG <=======

            var options = state.currentData as List<DebugMenuOption> ?? new List<DebugMenuOption>();
            options.FirstOrDefault(o => o.Hash() == currentHash).method?.Invoke();
	        
	    executingListerAction = false;                                    // =======> RESET FLAG <=======
        }

        ...
    }
    ...
}

DebugMenuOptions actions are executed via debug commands, and the method Targeter.BeginTargeting - which I’ve patched and intend to call only in this specific context - is invoked within one of them.

I want to use this flag to guard my patch instead of relying on reflection.
The example I shared above is simplified.
In the actual commit, I’ll move the flag setting into a separate method and wrap it in a try/finally block. This makes the guard more specific to this case and more future-proof by ensuring the flag is reliably reset immediately after the DebugMenuOptions action.

Idea B

Another idea could be to check whether the currently executed cmd type is CommandType.DebugTools. I haven’t looked into it in detail yet, but it should work, shouldn’t be too hard to implement, and might be useful in the future.

[notfood]

Idea B sounds really promising.

[notfood]

Good to go?

[Tick-git]

Yep it is good to go, I just tested it quickly again merged into dev.

While testing this the PR, I encountered consistent crashes when setting the raid points above approximately 750. This issue occurs even in an unmodded version of the game and has been confirmed by SirWilliams.

I haven’t yet found an existing bug report about this on the RimWorld Discord, though I didn’t search extensively. If no report surfaces, I plan to report the issue myself.

This still happens. I did not report it so far. I've written it down this time - so I won't forget it.