Sekiro framerate fix requires working framelock feature

SvdB-nonp commented 1 year ago

Not really a new issue, just a confirmation of the issue already mentioned in the Sekiro trainer.

Issue:

The feature Fix game speed for framelock is currently broken as the game crashes when enabled. According to the comments in the file this is due to tuxtrain actually writing past the region that is specified:

# NOTICE: In my experience this just crashes the game, I don't think this is correct.
# According to this: https://github.com/Lahvuun/sekirofpsunlock/blob/1ccc98c0de847dd0c1837381ef6e05ea423b8d2d/src/fps.c#L142
# It finds the pattern at pos 5376960685 just like this does, but then
# proceeds to write all the way over at pos 5421699608 instead.
# More research needed.

Effect

So I got into some Sekiro lately just to see how important this feature is and I noticed that past 120 FPS the game really does require the fix for the framelock.

120: Wolf runs somewhat slower than usual, which can be annoying/restrictive, but the game is playable in that state.
165: Wolf will move at roughly half speed without the fix, so pretty much unplayable.

Trial and error

I have tried playing around with the region and reading a bit in the other projects (see also them mentioned in issue #4 ). However I cannot prevent the game crashes and hence I assume the program always will keep writing more than it should. Probably there is little options working around the issue through the toml file and it requires a code change. Although I am clueless where this is going wrong.

Log does not give a clue, looks correct:

==> Sekiro Trainer 0.0.3
    Set FPS Framelock
      - Location: 5387236685
      - Found: C743188988883C4C89AB
      - Wrote: C743188988083C4C89AB
    Disable 16:9 aspect ratio lock.
      - Location: 5369932904
      - Found: 85C97447478B94C71C0200004585D274
      - Wrote: 9090EB47478B94C71C0200004585D274
    Fix game speed for framelock
      - Location: 5376960685
      - Found: F30F58D00FC6D2000F51C2F30F590518AAAA020F2F
      - Wrote: F30F58D00FC6D2000F51C2F30F59050000A8420F2F

Last attempt was to literally use the values from the log to try to force a more specific location, as shown below, but that has no other effect unfortunately.
(I am not sure where the value 00 00 A8 42 came from, but I just kept using that)

[[feature]]
name = "Fix game speed for framelock"
region = [5376960685, 5376960706]
pattern = "F3 0F 58 D0 0F C6 D2 00 0F 51 C2 F3 0F 59 05 18 AA AA 02 0F 2F"
replace = "F3 0F 58 D0 0F C6 D2 00 0F 51 C2 F3 0F 59 05 00 00 A8 42 0F 2F"
enable = true

leaty commented 1 year ago

Thanks for writing this up.

The feature Fix game speed for framelock is currently broken as the game crashes when enabled. According to the comments in the file this is due to tuxtrain actually writing past the region that is specified:

I think you might've misread the comment a tiny bit, it's the other way around as far as I could tell when I looked into how sekirofpsunlock did it.

It looks like it (sekirofpsunlock) is reading/writing at different locations, tuxtrain doesn't really have any feature to do this yet unfortunately. I'd need to fully look into it to find a good way to solve it because I think I saw the writing pos is based on something found at the reading pos making it more complicated than a simple search/replace.

SvdB-nonp commented 1 year ago

You are right, then I indeed misread that part, thanks for clarifying :)

I do see that they are using some sort of matrix to determine the closest valid value, around half the framerate, but I don't know really how that works and how to derive the hex value from the result. Otherwise we could have a few hardcoded values to play around with before doing structural changes.

leaty commented 1 year ago

No worries, now adding a way to write at a specific pos would be easy enough, something like:

put = [5421699608, "OF 59"]

Could also add a way to get specific values for put to write like so:

pattern = "F3 0F 58 __ 0F C6 __"

# Get all with ** based on pattern and combine
# Value would become "OF C6" here
get = "__ ** __ __ __ ** __"

# Put using get value (due to missing second value)
put = [5421699608]

Maybe even use regex in some form with support for grouping.

But if we need a generic way of yanking and converting/modifying/using values found for put to use, it gets a lot more complicated because it starts getting into having almost actual code in toml format. So it all really depends on what's required for this feature to work, and I've not yet looked closely enough.

leaty commented 11 months ago

@SvdB-nonp I just want to provide a (late) update on this, I'm in the midst of finding a place to move to and some other stuff. I will try to look at this after some personal life issues are dealt with. Hopefully you were able to use sekirofpsunlock to play in the meantime.

I do see that they are using some sort of matrix to determine the closest valid value, around half the framerate, but I don't know really how that works and how to derive the hex value from the result. Otherwise we could have a few hardcoded values to play around with before doing structural changes.

This in particular wasn't the issue I found, I think we could workaround that as you said, but to me it looked like the actual writing position was based on something found in memory. I'll get back on this eventually.

Also for more complex issues as I mentioned in the previous comment, do you think calling scripts would be a good idea? As in copy from memory -> pass to and run script -> get back output and position of which to write, I think it could alleviate from making tuxtrain too complicated, while allowing extreme customization for rare cases.

SvdB-nonp commented 11 months ago

Also for more complex issues as I mentioned in the previous comment, do you think calling scripts would be a good idea? As in copy from memory -> pass to and run script -> get back output and position of which to write, I think it could alleviate from making tuxtrain too complicated, while allowing extreme customization for rare cases.

Without considering rust-specifics, yes this sounds like a solid plan. The toml can stay relatively simple, and hence readable by users, with a reference to a rust script elsewhere doing the lookup needed.

It sounds it could also be a nice basis if other games use the same mechanic later on.

leaty / tuxtrain