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

Build with latest nightly (mostly) #14

Closed
wants to merge 4 commits into from
Closed

Conversation

simias
Copy link

@simias simias commented Dec 26, 2014

I wanted to steal some code for my gameboy emulator but I realized that this code hadn't been updated in a while so I decided to give it a try.

Most of the breakage was due to changes in rust syntax (nampespaced enums, no automatic Copy etc...) however the audio interface to SDL2 seems to have changed quite a lot which means that the current code is completely broken.

In particular rust won't let me declare a static AudioDevice anymore because it implements Drop. I'm not sure how to fix that particular issue without rewriting a big chunk of code so I left it commented for now. If you have any clue what to do I can look into it some more.

But besides that it builds correctly with only one warning left that I can't quite figure out (it looks bogus to me but I might be missing something).

When I tried to run it with mario bros the game seemed very slow even though it wouldn't take all of my CPU, maybe there's a timing issue, I'm not sure (and since I never ran this emulator before I'm not sure how it's supposed to look like).

So, it's not finished but hopefully it'll be useful.

@simias
Copy link
Author

simias commented Dec 26, 2014

I just found out that @nukep did the same thing back in november...

I integrated his commit for the audio workaround, unfortunately I encounter a new error that doesn't make any sense to me or anybody on IRC:

audio.rs:91:19: 91:43 error: mismatched types: expected `core::option::Option<extern "C" fn(*const libc::types::common::c95::c_void, *const u8, i32)>`, found `core::option::Option<extern "C" fn(*const libc::types::common::c95::c_void, *const u8, i32) {audio::nes_audio_callback}>` (expected fn pointer, found fn item)
audio.rs:91         callback: Some(nes_audio_callback),
                              ^~~~~~~~~~~~~~~~~~~~~~~~

Need help.

@nukep
Copy link

nukep commented Dec 28, 2014

Well, that's bizarre.
rust-sdl2 had the same problem, which was recently fixed in Rust-SDL2/rust-sdl2#244. The problem seems to "go away" with a cast to the function pointer, as such:

        callback: Some(nes_audio_callback
            as extern "C" fn
            (userdata: *const c_void,
                stream: *const uint8_t,
                len: c_int)),

There's no way that this is the way it's supposed to be done, though. It has to either be a regression in rustc, or there's simply a better way without casting.

With all that said, direct use of ll should be temporary anyway. I think a temporary hack like this should be acceptable until the Rust abstractions are used in its place.

@simias
Copy link
Author

simias commented Dec 29, 2014

Agreed but I wonder if an issue should be opened on the rust bugtracker? That looks fishy to me.

@nukep
Copy link

nukep commented Dec 29, 2014

Ah, this seems to be the reason: rust-lang/rust#19891

Explicit casting to a fn pointer seems to be the only option right now, until there's a (possible) fix. :(

@simias
Copy link
Author

simias commented Dec 29, 2014

Thanks for taking the time to look that up! I fixed it in the latest commit.

The only thing that remains is this weird warning:

ppu.rs:809:78: 809:90 warning: the `color:` in this pattern is redundant and can be removed, #[warn(non_shorthand_field_patterns)] on by default
ppu.rs:809                 (None, Some(SpriteColor { priority: SpritePriority::BelowBg, color: color })) => color,
                                                                                        ^~~~~~~~~~~~
ppu.rs:810:75: 810:87 warning: the `color:` in this pattern is redundant and can be removed, #[warn(non_shorthand_field_patterns)] on by default
ppu.rs:810                 (_, Some(SpriteColor { priority: SpritePriority::AboveBg, color: color })) => color,
                                                                                     ^~~~~~~~~~~~

It's weird because while the color is not used to discriminate between the matches but just to bind it to a local variable. On the other hand the match is pretty complex so maybe I'm missing something.

@simias
Copy link
Author

simias commented Dec 29, 2014

Also, the games are still super slow, is there a regression?

@simias simias closed this Nov 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants