-
Notifications
You must be signed in to change notification settings - Fork 4
Elmish proof of concept #27
base: master
Are you sure you want to change the base?
Conversation
@MangelMaxime I added Elmish, React and Fulma to the project. In order to it all work I added some packages like style-loader, css-loader etc... But I am neither sure all the packages I added are needed there nor am I am confident about the webpack config update. Since I have no other documentation than source code I think there are changes that may not be relevant to the configuration. Could you check on your side please? Thanks! |
For the note, it's a little bit more than an Elmish-Pixi app. It's more a Elmish-React-Fulma+Pixi app. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only read the code for now. I will test it and review the implementation later during this weekend.
public/elmish/index.html
Outdated
<head> | ||
<meta http-equiv='Content-Type' content='text/html; charset=utf-8'> | ||
<meta name="viewport" content="width=device-width, initial-scale=1"> | ||
<script src="https://cdn.polyfill.io/v2/polyfill.js?features=default,fetch"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this scrip tag because we now include core-js
by default. And we don't need fetch :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
public/sass/main.sass
Outdated
@@ -0,0 +1,428 @@ | |||
// ******************************************************* TODO **************************************************** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there some cleanup need in this file. I guess you took it from you website for "casque noir"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I really just use this file for importing bulma. So maybe I could just not use this file and import bulma css directly if that's possible?
src/elmish/App.fs
Outdated
open Fable.Helpers.React.Props | ||
|
||
// Fulma css | ||
importAll "../../public/sass/main.sass" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to use importSideEffect
in general when importing file to create a side effect in webpack. (for info importSideEffect is "new")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
Thanks for taking time to review this PR @MangelMaxime! |
Ok this seems to works, I made some really little improvements. I thinks we could improve the comments and also explain more how everything is wired. My problems, is the GameModel is using mutable and so I believe we can be out of sync between the Elmish app and the Pixi rendering state. This week-end I am planning to work on Elmish eco-system and Fulma. And then, I want to create a "real case" game using Fable and Pixi so we can use it to test this architecture or any future proposition. (Should be something equivalent to what fulma-demo is for Elmish + Fulma)). |
Just because, I forgot to conclude :) I would probably wait a little before merging this PR. |
Well I've been thinking about the dependency between Elmish and Pixi. I think that using an immutable field for updating the game state is just a matter of trust. The kind of trust we would need for instance when getting an update from a Suave server and vice-versa. Regarding the Model managed by Elmish and shared with the pixi rendering loop, there I think is a dangerous thing. It's not its mutability which is dangerous, but the fact that elmish and pixi somehow try to share a reference to the same model. And there, yes, out of sync can be a problem. Now, all things considered, Since Pixi now manages its rendering loop - no need to call RAF - I think we should let pixi manage its own model, sprites, an all but use Elmish as a Controller. What I mean, is that I have been trying to model a real life sample where the out-of-sync could be a problem. Here it is: a GameOver Rule. Let's say that when we have more than 50 dragons displayed on screen it triggers a GameOver. Who's in charge there? Elmish or Pixi? I think that Pixi does only need to In this scenario, Pixi does not have any idea of the current state of Elmish model. It's not its business anymore. Then Elmish, acting as a controller, will get messages from Pixi, updates its model and eventually tell pixi what to do by updating its state. So in the end the only dirty call we'd get would be: So with our dragons, it's Elmish who would be in Charge of testing the GameOver case and tell pixi to play the GameOver animation. No more shared model. Only Pixi sending messages to Elmish and Elmish updating pixi state. Maybe I'm missing something, but I think it could be a very simple somehow cleaner compromise. What do you think? |
Seems like a good idea indeed. Would you like to try it in this PR? |
Ok I'll make the changes. 👍 (Not exactly sure when though 😉 ) |
I need to look this into more detail. Quick thoughts: Goal: Adapt Elmish to a Game with Pix as RendererFor this let's check how a productivity app works and how it differentiates with a game:
While in a game
I think this is very similar to what the Elmish.Worker helper does, so I will try to adapt it. The key is to let Elmish control when ticker.autoStart = false;
ticker.stop();
function animate(time) {
ticker.update(time);
renderer.render(stage);
requestAnimationFrame(animate);
}
animate(performance.now()); Another idea to increase performance could be not to call About mutable fields, ideally there should be none, but this is difficult in a game with too many updates (there would be too many memory allocations that would drop performance) and also using Pixi sprites, because at the . Anyways, the only thing we have to ensure is the model is not touched outside the |
I'm not sure to understand what it is :) |
This is a proof of concept of bi-directional communication between a Pixi application and an Elmish application. original discussion