Fix timing regression affecting ES_LAUNCH. #3811
Merged
+1
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Scheduling an event for zero cycles in the future actually means zero cycles with new timing changes, but the code for IPC ACKs was depending on it meaning "soon". Fixes dolphin-emu#9511. I'm not at all confident this is actually right... but it seems to work.
|
Yeah, this bug probably pops up in other places too. |
|
Fixes the issue. This is why we merged that early. There are probably tons of "soon" that are going to be instant now and break things. |
|
FWIW there are a bunch more |
|
Or we just change this function to schedule for the next block. Calling the event handler directly doesn't need a CoreTiming wrapper :/ |
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
May 7, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
May 7, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
May 7, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
May 7, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
May 8, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
No, because before #3800 the interrupts were being delayed to the next slice, which could be anywhere from 1 to 20,000 cycles depending on what other events were scheduled.
Note: Numbers have been simplified. The size of a block is technically unlimited, but generally they aren't that large. |
|
Can this please be reviewed. We cannot leave ES_Launch broken for 5.0 |
|
Oh, I forgot to say LGTM on this. |
|
Reviewed 1 of 1 files at r1. Comments from Reviewable |
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 1, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 6, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 9, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 11, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 15, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 18, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 19, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 20, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
leoetlino
pushed a commit
to leoetlino/dolphin
that referenced
this pull request
Jun 23, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
Jun 26, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
magumagu
added a commit
to magumagu/dolphin
that referenced
this pull request
Jun 26, 2016
Fixes issue 8328. As far as I know, this works the same way as console. Games will generally react to the reset button the same way as Home->Reset, so this is only marginally useful, but possibly nice to have in certain situations. Note that if you try to use Reset, and you're running a WAD which isn't installed, it will likely crash because WADs respond to the reset button by launching themselves with ES_LAUNCH. It might be a good idea to add some sort of hack to make this work as expected. It would be easy to extend this to support the power button, but it's unclear how exactly that should be exposed in the UI. See also issue 8979. Needs to be rebased once PR dolphin-emu#3811 is merged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Scheduling an event for zero cycles in the future actually means zero
cycles with new timing changes, but the code for IPC ACKs was depending on
it meaning "soon".
Fixes #9511.
I'm not at all confident this is actually right... but it seems to work.
This change is