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

Move Immediate XFB down in DTM header #6207

Merged
merged 1 commit into from Nov 19, 2017

Conversation

JosJuice
Copy link
Member

7f0834c, which added the Immediate DTM setting, also added a byte to DTM headers for Immediate DTM. There are two problems with this:

  1. The byte was added in the middle of the header, throwing off the offsets of several other settings. There are programs other than Dolphin itself that parse DTM files, so this is not acceptable.

  2. If I'm not mistaken, Immediate XFB doesn't affect sync. Putting it in the DTM header unnecessarily prevents people from using a different value for that setting than what the movie author used.

I would appreciate if this could get reviewed quickly so that TASers don't have time to create DTMs with broken headers.

@JosJuice
Copy link
Member Author

cc @iwubcode – is not syncing this setting fine?

@JosJuice
Copy link
Member Author

Stenzek and I reached the conclusion that while changing the setting doesn't affect the sync of the emulated system, it affects the frame and lag counters in Movie.cpp, which can lead to some unwanted consequences for TASing. So this PR now just moves the setting instead of deleting it from the header.

@JosJuice JosJuice changed the title Remove Immediate XFB from DTM header Move Immediate XFB down in DTM header Nov 19, 2017
@JosJuice
Copy link
Member Author

Wait a little... The XFB to RAM setting is wrong too >.<

@iwubcode
Copy link
Contributor

@JosJuice - thanks, sorry I didn't know the ordering of this stuff mattered...maybe we need a comment? (or maybe I missed it!)

@JosJuice
Copy link
Member Author

JosJuice commented Nov 19, 2017

It should all be fixed now. I've added a comment too.

Fortunately, the number of non-XFB settings that were affected was actually much smaller than I thought it was after only seeing the immediate mode commit.

7f0834c moved the locations of the Real XFB (now XFB to RAM) and
Disabled XFB (now Immediate Mode) settings. There are programs
other than Dolphin that parse DTM headers, so this is not good.

Note that Immediate XFB actually is the inversion of Disabled XFB.
I hope that's not too much of a problem...
@@ -52,6 +52,8 @@ struct ControllerState
static_assert(sizeof(ControllerState) == 8, "ControllerState should be 8 bytes");
#pragma pack(pop)

// When making changes to the DTM format, keep in mind that there are programs other
// than Dolphin that parse DTM files. The format is expected to be relatively stable.

This comment was marked as off-topic.

This comment was marked as off-topic.

@leoetlino leoetlino merged commit 28f9034 into dolphin-emu:master Nov 19, 2017
@JosJuice JosJuice deleted the dtm-immediate-xfb branch November 19, 2017 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants