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

FRP interface reactiveOf #486

Closed
wants to merge 5 commits into from
Closed

FRP interface reactiveOf #486

wants to merge 5 commits into from

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented May 22, 2017

This patch allows users to create interactions by defining a FRP
program, based on the reflex library. The dependency on reflex is
optional, and enabled with -freflex.

This is a result of some hacking with the reflex people at the Compose
Unconference (but all the code in this patch is mine).

A mode where even reactive collaborations are supported depends on some
fancy new feature in reflex, which would allow stats to be reset.

This might be of interest to @ryantrinkle and the other reflex guys.

@nomeata
Copy link
Contributor Author

nomeata commented May 22, 2017

@ali-abrar

Copy link
Collaborator

@cdsmith cdsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@@ -36,6 +40,12 @@ Library
cereal >= 0.5.4 && < 0.6,
cereal-text >= 0.1.0 && < 0.2

if flag(reflex)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think a flag is the right answer here? What exactly is the goal?

  • If it's the additional dependency, I don't have any concerns about it. I'm happy to just depend on reflex without the flag.
  • If you're trying to dissuade usage until you're more confident in the API, consider a WARNING pragma instead? Then people could still try it out on the live site.
  • If it's about polluting the API, consider a different public module, like CodeWorld.Reactive, instead?

Or is there some reason we should be worried about pushing this live?

@cdsmith cdsmith changed the title FRP interface reativeOf FRP interface reactiveOf May 22, 2017
@nomeata
Copy link
Contributor Author

nomeata commented May 22, 2017

If it's the additional dependency, I don't have any concerns about it. I'm happy to just depend on reflex without the flag.

That was my worry. But if that is not a problem, I can remove it.

Caveat: I have built reflex from github. I guess I should test if this actually works with reflex from hackage…

I think the interface is pretty much the only sensible (the other option would be a Behaviour Time, but Ryan advised against it).

I am also less worried about polluting the API, it is the “advanced” API anyways.

@ryantrinkle
Copy link

I'll get the new version of Reflex up on hackage ASAP. I'd recommend holding off on upstreaming this until then, as there have been some breaking changes.

This patch allows users to create interactions by defining a FRP
program, based on the reflex library. The dependency on reflex is
optional, and enabled with -freflex.

This is a result of some hacking with the reflex people at the Compose
Unconference.

A mode where even reactive collaborations are supported depends on some
fancy new feature in reflex, which would allow stats to be reset.
to keep CodeWorld/Driver clean (and to give @ryantrinkle an idea about
what kind of interface I would have preferred here :-)).
@nomeata
Copy link
Contributor Author

nomeata commented May 22, 2017

I'd recommend holding off on upstreaming this until then, as there have been some breaking changes.
Ok, will do . I changed the version range to require 0.5.

Ryan, f93f42c has a sketch of the simple interface I would like to see. The downside is that it hard-codes the number of events and behaviors, and I am temped to pull out heterogeneous lists to fix that … but then it would not longer be simple…

@ali-abrar
Copy link

Given that events can be merged and fanned out, maybe you can get away with just one event for the simple host example.

@nomeata
Copy link
Contributor Author

nomeata commented May 22, 2017

I was wondering: Is there a performance penalty of merging and fanning out?

Also, this eventTriggerRef can contain a Maybe. If this is Nothing, does this mean that nobody is listening to this event? If CodeWorld could see that the time event is not used, it could avoid looping (it already does that in the other drivers, by some tricks involving comparing StableNames).

But that would break if the two events were merged and fanned out, right?

@ryantrinkle
Copy link

ryantrinkle commented May 22, 2017 via email

@cdsmith
Copy link
Collaborator

cdsmith commented May 22, 2017

@nomeata @ryantrinkle Is there a fundamental reason why making time a Behavior is a bad idea? It seems like the obvious choice to me. Is this just a matter of the implementation being awkward, or is there something wrong with the abstraction?

@ryantrinkle
Copy link

Defining time as a Behavior has strange semantics in some subtle ways. The most important of these is that many of the things you'd like to define won't work if time is a Behavior - in particular, you can't do something like "Fire an Event when this Behavior crosses 0" and then invoke it on now - myDeadlineBehavior, because Behaviors are pull-driven, and thus can't spontaneously push things.
It would also require (much) more information about the function the Behavior represents than we actually have. Discontinuous functions would be quite challenging to deal with.

Now, you could certainly accumulate an Event t DiffTime (or similar) to create a Behavior t Time, but this Behavior would move stepwise, which might not be expected. Also, the natural way in Reflex would be to have the Behavior represent the last frame's time, not the current frame's time, which might be confusing for some users. It would be possible to update the Behavior in one frame, and then fire the delta time Event in another frame - Reflex guarantees that Behavior updates like that aren't observable by the program, so I think the semantics would work out. This would probably be the way to go if you wanted to provide now :: Behavior t Time. The Event t DiffTime would still need to be provided, so that local state can be updated and things like that (Behavior changes can't cause that in Reflex).

@cdsmith
Copy link
Collaborator

cdsmith commented May 22, 2017

Okay, I see. Yeah, I guess if you had a Behavior for time, then you'd also need an oftenEnough :: Event t () that fires every frame, so you could start from that and filter down to an Event that captures important time thresholds. And now by trying to simply one of the two components, we've ended up with three components instead...

@ryantrinkle
Copy link

@cdsmith Yep! Eventually, if we did a whole lot of work on continuous-time FRP (which Reflex is designed to be consistent with, as an extension), we might be able to provide something cleaner. However, I think making continuous time FRP nice to work with is still an open problem, and Reflex's approach of "time is just a totally-ordered set" seems to be great for most kinds of applications.

@cdsmith
Copy link
Collaborator

cdsmith commented May 26, 2017

@nomeata Do you prefer to merge this, or wait for Reflex changes? If the former, I'd prefer not to define a module in the Reflex hierarchy here. If that's just interim and you don't want to merge this, that's fine. Just checking.

@ryantrinkle
Copy link

Oh, yeah, let's get Reflex.SimpleHost into Reflex. I'm getting close to having 0.5 ready for release, and I think I could stick that in quite easily. @nomeata Thoughts?

@nomeata
Copy link
Contributor Author

nomeata commented May 26, 2017

There is no reason to hurry with this PR, so moving this module (or something similar) to reflex first, and then adjusting this branch to build on the release 0.5, makes sense.

@cdsmith
Copy link
Collaborator

cdsmith commented Oct 9, 2018

@nomeata @ryantrinkle

I have a renewed interest in merging something like this now. I'm interested in turning http://code.world/haskell into a more general-purpose UI for building Haskell code with commonly used APIs. The first three on my list are gloss, reflex, and diagrams.

Any thoughts on the right path forward here? There was some discussion about making changes to reflex to make this easier, and of this not working correctly with reflex. I'm happy to poke around a bit myself, but I may as well ask first.

@nomeata
Copy link
Contributor Author

nomeata commented Oct 9, 2018

I think the question was just whether simpleReflexHost ought to live in reflex, but I don’t think you should be held up by this.

@hpacheco
Copy link

I have a renewed interest in merging something like this now. I'm interested in turning http://code.world/haskell into a more general-purpose UI for building Haskell code with commonly used APIs. The first three on my list are gloss, reflex, and diagrams.

@cdsmith I had already been doing some hacking on the gloss front: https://github.com/hpacheco/codeworld-api-gloss

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.

5 participants