Closed marshallpierce closed 6 years ago
Thanks for opening this pull request! I am attempting to restart the errored jobs that probably shouldn't have failed but Travis is currently giving me some issues. I'll try again later.
I am unfortunately quite busy this week and this weekend, but I will try to get back to this the week of the 8th. Really appreciate all the work you've put into this!
@marshallpierce My apologies for making you wait for so long. I unfortunately have a couple of things I need to get to in both turtle and my personal life before I can give this PR more attention. I don't have an exact timeline yet, but please know that I do have this mind and will get to it as soon as possible in the coming weeks. :)
Hi @marshallpierce, I'd like to personally take responsibility for the stagnation of this PR. Looking back, I should have asked you to make smaller incremental changes that are faster to review. Taking the time out to review a huge set of changes like this really hasn't been easy. Going back and forth making adjustments will take a lot of time too.
Since that is totally on me, and I don't expect you to go back and break down your changes into small increments (that's a lot of work), I'll do everything I can to do the work for you. I'll bring in your changes slowly (using your commits so the work is still tagged with your name) and keep as much of it as I can.
Here's what's been on my to-do list that's been preventing me from looking at this:
This is quite a lot and I was hoping to finish it sooner, but life has gotten in the way. This WebAssembly work would fit in there around right after I finish writing the guide and before releasing 1.0.0 since that is the point where I would really have confidence that the API is good and stable.
Anyway, like I said, this doesn't really require anymore work from your end. Once I'm ready and I've finished those other things I need to work on, I'll come back and address all of this.
Thanks again for your work! The time you spent figuring out how to do this is very valued and appreciated! Sorry I didn't do a good enough job of steering you in a direction that would have helped get this merged sooner.
OK, I'm glad it won't die. :)
@marshallpierce Thanks again for all your work on this! I learned a lot from this experience and like I said, I'll be making sure that I am more responsible about getting more manageable PRs in the future. So sorry that this wasn't a more positive experience for you!
Have an excellent weekend! :)
This introduces a new
Runtime
type. This encapsulates all the platform-specific parts of running Turtle logic: communication between turtle logic and rendering backend, how grahpics are actually rendered, PRNG, timekeeping, etc.DesktopRuntime
simply uses the existing code, rearranged a little bit so that the desktop-specific bits are indesktop
(e.g.Server
now hasfrom_piston_event
,update_window
and the render loop fromRenderer
since those are strongly tied topiston-window
things).Runtime
for both canvas and desktopGraphics
that writes to a RGBA pixel buffer as needed by a<canvas>
.desktop
module, with analogouscanvas
moduleRuntime
Graphics
so that it can work with either desktop or canvas flavorsmain()
. ATurtle
is initialized differently for desktop vs wasm use, so we need to hide that from users. Also, webassembly has to provide some parameters when it passes control flow to user logic, so it can't just re-usemain
or something like that. This also prevents users from ever doing startup wrong.Turtle
type so that users will always have a safe PRNG accessible without worrying about platform issues. Until Rust gets NLL in stable, this requires an extra local var in certain uses (e.g.turtle.set_color(turtle.random::<Color>())
doesn't work yet without extracting a local for the color).Todo:
N.B. I think the names
RenderProcess
andServer
aren't as helpful as they could be for guiding a reader. TheRenderProcess
is the thing that isn't the render process, for instance: it's more of a client library for working with the other process.Server
might be calledRenderWorkerProcess
or something like that; it's not really serving anything. Anyway, now that startup is done via a macro, the fork model could be moved back to a multithreaded model easily enough and then the whole issue goes away.