fixed the bug where continue backwards did nothing.

[japjitsingh]

This commit resolves #14.

The bug relates to calling Cursor::ReplayBackward with -1 for the stepCount argument. The C++ code examples in the ttd-bindings repository mistakenly calls Cursor::ReplayForward with -1. It should be called with -2. Although Cursor::ReplayForward works normally, using the same argument for Cursor::ReplayBackward does nothing.

The fix can be confirmed by looking at the code for Python bindings in the ttd-bindings repository.

It correctly adds the following line: m.attr("MAX_STEP") = 0xFFFFFFFFFFFFFFFE; which as an int (later casted to an unsigned long long) equates to -2.

Any calls to continue code execution is then achieved by using _pyTTD.MAXSTEP as seen in example_api.py.

[japjitsingh]

Hello @simsor, @citronneur

I accidentally reverted my second commit, so we see 3 new commits. (2nd commit, revert, 2nd commit) Sorry about that.

The latest commit resolves #12 which occurs when stepping backwards, and mildly improves code readability and design.

As per my understanding of the plugin, when stepping backwards:

• TTD Engine is used to go one instruction back.
• A Breakpoint event is raised to signal to IDA to update its UI.
• IDA complains about an unknown breakpoint as there is no breakpoint at the arrived-at instruction.

In terms of original code implementation, the stepping backwards functionality is achieved by the following steps:

• BackwardSingleStepRequest::activate() sets m_backwardsSingleStep to true and calls continue_process().
• The event-handler DebuggerManager::onResume() is triggered:
  • If m_backwardsSingleStep is true, m_isForward is set to false, and applyCursor(1) is called.
  • A breakpoint event is raised which simulates hitting of a breakpoint at the arrived-at instruction.
  • The original value of m_isForward is restored, m_backwardsSingleStep is set to false (as its default value) and the function returns.
    • BackwardSingleStepRequest::activate() returns.

The code centering around m_backwardsSingleStep seems a bit of a "hack". I have introduced some changes to the code that eliminates m_backwardsSingleStep while keeping all functionality intact.

The changed code flow is as follows:

• BackwardSingleStepRequest::activate() toggles m_isForward, explicitly sets m_resumeMode to resume_mode_t::RESMOD_INTO and calls step_into().
  • The event-handler DebuggerManager::onResume() is triggered:
  • The switch statement matching resume_mode_t::RESMOD_INTO is taken, thereby calling applyCursor(1).
  • A step event is raised.
  • m_resumeMode is set back to its default value of resume_mode_t::RESMOD_NONE, and the function returns.
• BackwardSingleStepRequest::activate() toggles m_isForward again and returns.

Some other changes that were made to ensure proper functionality:

• BackwardStateRequest::activate() explicitly sets m_resumeMode to resume_mode_t::RESMOD_NONE.
  • This is technically not needed as m_resumeMode is previously set to resume_mode_t::RESMOD_NONE anyway.
    • Only done to improve code comprehension.

In conclusion: the commit gets rid of m_backwardsSingleStep and significantly simplifies the DebuggerManager::onResume() function. Instead of relying on a boolean value, backward-step-into simply changes the direction of the execution and uses the same code as that of the normal step-into. Breakpoint event is not used and we will not get the "unknown debugger breakpoint" errors while stepping backwards.

Thank you for the great plugin, and please keep up the astounding work! :grin:

Japjit