Cxbx-reloaded: Regressions in master

Created on 31 Aug 2017  路  6Comments  路  Source: Cxbx-Reloaded/Cxbx-Reloaded

Recently, master started to show a few regressions. They where first discovered when testing pr #696 (see https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/pull/696#issuecomment-326358253).

Luke discovered the same regressions appear in master. He suspects the recent OOVPA review commits are the cause.

For this issue,

  • Copy over the linked regression report to here
  • Discover which commit cause the regressions
  • Fix them ;)

Once that's done, we can take a look at pr #696 again.

help wanted high-priority needs-verification regression

All 6 comments

ZSNES regression happened on eddea78768b9b33396b674b37563b810a8bb20c2 as it is working here with 00c833ed962fbd7a675db4f155906204f86c56e7
HLECache.zip

capture

ZSNES regression is caused by GetOverlappedResult function probably.
If the function succeeds, the return value is nonzero. However, the return value is 0 by ZSNES.

[0x1790] EmuXapi.cpp : EmuPatch_GetOverlappedResult(
   hFile                : 000004B4
   lpOverlapped         : 0AECC614
   lpNumberOfBytesTransferred : 0x8CEFE28 (*value: 0x3E55FC)
   bWait                : FALSE
);
[0x1790] EmuXapi.cpp : EmuPatch_GetOverlappedResult returns 0

Note: In the past commit it was registered only on XDK 4627.

GetOverlappedResult just calls WaitForSingleObject internally, I don't think it requires a patch.
Please try commenting out the FUNC_EXPORTS line within that function.

Update: I can confirm, GetOverlappedResult does NOT need to be patched, at all.
Please keep the OOVPA, but remove the function implementation from EmuXAPI. It might be best to put an #if 0 around it.

We want to keep the OOVPA, and as many as possible, even for functions we don't patch, because they help with reverse engineering and debugging.

Further down the line, we may even be able to log params and return values of functions that we don't patch, so the OOVPAs will stay useful!

Thank you for your confirm! GetOverlappedResult leave unpatched.

Closing this as we track regressions such as this in the game compatibility repo recently.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

PatrickvL picture PatrickvL  路  4Comments

LukeUsher picture LukeUsher  路  4Comments

Kumoashi picture Kumoashi  路  3Comments

childishbeat picture childishbeat  路  4Comments

PatrickvL picture PatrickvL  路  3Comments