libretro / snes9x

Snes9x - Portable Super Nintendo Entertainment System (TM) emulator
http://www.snes9x.com
Other
47 stars 57 forks source link

Remove this hack... #165

Closed ghost closed 6 years ago

inactive123 commented 6 years ago

This is a beneficial hack. I don't want to merge this without consulting others first.

I have tried leading from behind on these issues but I don't really like how all of the additional features which are actually good are being removed from the libretro core just to make it more similar to upstream. I can only hope that when all is said and done, these features will still exist on the core that will be provided on the buildbot, since I am not prepared to sacrifice 'hacks' like this and the CPU overclocking either. Users should be able to do that with the mainline Snes9x core they download instead of having to download yet again another core, this all becomes too confusing.

Tatsuya79 commented 6 years ago

It's hard to ignore when someone comes and says something bad about the work you're doing for free on your own time just because of suspicion. We shouldn't write too hastily something negative about a PR that would need some testing and eventually fixing first before considering if it's actually worth merging or not.

If it's about Radius I think he didn't know about the mid-frame limitation and that he has nothing against this hack in the end.

inactive123 commented 6 years ago

Let's not have a huge back and forth over this like in the previous thread.

I am fine with the rebase ideas suggested by @fr500. My only condition is that all the core options and additions that are currently in the libretro port, will remain in the core that people will be able to download on the buildbot when all is said and done. In case it needs any reiteration, I am not OK with sacrificing CPU overclocking or the Chrono Trigger midframe hack in the eventual finished libretro core. These are good features to have, and I don't want to sacrifice them or require users to download yet another separate core in order to use these features. So if the buildbot will build from a branch that has these additional features and this branch will be what the buildbot compiles, then that is fine. That is how it was explained to me anyway by @fr500.

Anyway, let @fr500 just do his thing and until then let's not merge anything right now that has to do with this. We will wait until the proper rebasing, we don't want to sacrifice beneficial features right now until the rebase is done.

andres-asm commented 6 years ago
because one of the maintainers reacted when i said that I dont like hacks, so why did I send PR for this hack......

I don't even know what's going on lol, I think I actually agreed with @retro-wertz here with regard to hacks that deviate too much from upstream.

andres-asm commented 6 years ago

there is really no need, my fork is gonna replace this one as soon as I'm done

On Sun, Jul 1, 2018 at 12:22 PM retro-wertz notifications@github.com wrote:

please reopen and merge, so this will be consistent with @fr500 https://github.com/fr500 repo. im sure it was very well tested by them and that this is indeed not needed

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libretro/snes9x/pull/165#issuecomment-401620432, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpC0FUY4rcYYuWNuKMx5ik2WkN_2MKaks5uCQVNgaJpZM4Uu7fw .

andres-asm commented 6 years ago

With there is no need I meant there is no need to do any changes here, not that your hack is not needed or something like that.

On Sun, Jul 1, 2018 at 12:35 PM retro-wertz notifications@github.com wrote:

yeah. im sure anyone who will still play this game will be so excited of this battle transition glitch. thanks for whoever tested this and said its not needed...

On Mon, Jul 2, 2018 at 1:29 AM, Andrés notifications@github.com wrote:

there is really no need, my fork is gonna replace this one as soon as I'm done

On Sun, Jul 1, 2018 at 12:22 PM retro-wertz notifications@github.com wrote:

please reopen and merge, so this will be consistent with @fr500 https://github.com/fr500 repo. im sure it was very well tested by them and that this is indeed not needed

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libretro/snes9x/pull/165#issuecomment-401620432, or mute the thread https://github.com/notifications/unsubscribe-auth/ ABpC0FUY4rcYYuWNuKMx5ik2WkN_2MKaks5uCQVNgaJpZM4Uu7fw

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libretro/snes9x/pull/165#issuecomment-401620812, or mute the thread < https://github.com/notifications/unsubscribe-auth/AWPDtpBBKIPrrEPfyVWLhfTXGBV_u-OAks5uCQbwgaJpZM4Uu7fw

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libretro/snes9x/pull/165#issuecomment-401621257, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpC0OVJZac9i31Satxiq98KyMY1yIapks5uCQh0gaJpZM4Uu7fw .