Slight inconsistency with set_memory_breakpoint()

[snemes]

The documentation for set_memory_breakpoint states that the restore argument "restores the original memory protection", for which the phrasing is a bit confusing.

The bpm x64dbg command describes the restore argument in its documentation as "restore the memory breakpoint once it’s hit", so it does not restore the "memory protection", but it restores the breakpoint after being hit (which is kind of similar because memory breakpoints are usually implemented using "guard pages", but it does not restore the "original" protection).

So passing a zero (0) value will basically create a singleshoot memory breakpoint (which is the default in x64dbg if you create the memory breakpoint from the "Memory Map" view by pressing the F2 key), while passing restore=1 as an argument will create a permanent breakpoint (which based on my experience is rarely what one wants for memory breakpoints, but this could be a matter of personal preference).

It would be more consistent if the function signature for the set_memory_breakpoint function could be more similar to the function signature of the regular set_breakpoint function, by using the term singleshoot for the argument instead of restore, and setting it to the False by default (i.e. the same way how set_breakpoint works right now).

E.g.:

    def set_memory_breakpoint(self, address_or_symbol: int | str, bp_type: MemoryBreakpointType = MemoryBreakpointType.a, singleshoot: bool = False) -> bool:

I can submit a patch if that helps.

[dariushoule]

Hey @snemes, agree that the wording here is misleading. I like the suggestion of lining up the signature to be more like set_breakpoint.

I'm waffling a little on the default for singleshoot. I'd wager the majority of cases people will be leaning towards wanting singleshots for memory bps as you mentioned, but with non-x memory breakpoints this might not be true. I think for consistency with set_breakpoint I'd lean towards defaulting False. If you have very strong feelings one way or the other I'm ears.

A patch would be welcome and appreciated! My thoughts on the approach would be the following:

1. Lets leave in restore as a kwarg, but deprecate it - making the docstring something to the tune of "deprecated, use singleshoot". I'd like it if existing scripts could keep working until I do a major/breaking release.
2. Lets add the singleshoot preferentially and keep the signature consistent with set_breakpoint.

[dariushoule]

Well actually if we leave in the deprecated arg any one who has used it positionally will have the behavior flipped. Might just need to bite the bullet on it, crap 😆