forcecraft / aion

An e-learning platform written in Elixir and Elm based on real-time gameplay
6 stars 0 forks source link

feature: Event logging in rooms #174

Closed mrapacz closed 6 years ago

mrapacz commented 6 years ago

Summary This PR adds event logging to Elm.

image

image

Done

I did not wanna focus on styling the logs properly right now. I think it should be part of a separate issue (especially that I think we should contact our friend-designer first). This PR only takes case of providing the logic.

So far, there are only last three events displayed (this can be tweaked easily to whatever we find useful later, here I only wanted to show that the whole logic works and didn't want the layout to jump etc.).

Things worth discussing and possibly adding follow-up issues for:

mrapacz commented 6 years ago

@pmrukot @jtkpiotr Check this commit's changes I started this PR by reorganising the current RoomChannel to be Channels.Room. I moved all of this module's dependencies to Channels.Room/. I also made the Presence and QuestionChronicle be supervised by a new Supervisor - Channels.Room.Supervisor. In general, I'm aiming at splitting up the Channels.Room module, because right now this is the central point of our backend and the file is now 200 LOC long and looking to get even longer with time. This could be done better I guess.

I also think we should rename the Aion.RoomChannel to be something else. I did the things above to start distinguishing between those two, because in fact, the RoomChannel doesn't have anything to do with actual channels, it's just a GenServer keeping the game data.

If you have any comments on this feel free to add them whenever you want, as I will be building this PR on top of these changes.

// also, the extra formatting I added is done by mix formatter as usual

if the #175 gets merged before this PR, the above changes won't be visible as part of this PR.

mrapacz commented 6 years ago

@pmrukot as for the formatting changes - this is why I wanted to merge the mix formatter PR sooner, to minimize the diff in here

pmrukot commented 6 years ago

Anyway you shouldn't use the formatter in other PRs before the official one is merged. Its a mess to see actual features. Now it does not matter.

mrapacz commented 6 years ago

Agreed