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

[New UI] Unity Build in React #616

Merged
merged 66 commits into from Jun 27, 2018
Merged

[New UI] Unity Build in React #616

merged 66 commits into from Jun 27, 2018

Conversation

OlafSzmidt
Copy link
Contributor

@OlafSzmidt OlafSzmidt commented Jun 4, 2018

When merging, merge this along with PR: ocadotechnology/aimmo-unity#211.

Some of the changes:

  • Docs changed
  • Parcel is now not minifying code as well as caching has been disabled.
  • Introduces 3rd party react-unity-webgl package.
  • Makes use of the helper function @mrniket wrote for snapshot testing in current tests.
  • We register an external listener for a function here SendAllConnect() and we dispatch calls to Unity's webGL build too.
  • New reducer, new store format, loads of new types.
  • API used in our epics for unity slightly changed. It's parameterised for a generic case but we left epics separate.
  • Little refactoring of players in aimmo.

Things that could be improved and we should discuss for future:

  • Error catching. Right now our React has very little error catching. Some of the stuff I've added here is also poorly caught. We sometimes use catchError() but even then we don't use the error object to use that information. Even worse, we can have a success action in redux being returned by the epic but when we emit a certain function call to Unity, SendMessage() call might fail and we have no way of catching that through the redux stuff?
  • Marble tests are probably incorrect for the epics. At first when I had two marbles, with events at frame 10 everything was fine. Now even when the first event, a, is dispatched at frame 10, the success response happens at frame 0. It also completes, which we probably don't want to happen. Unsure what's happening here. Tests passing right now because I emit the first event at 0 and expect a success and completion at 0 as well.
  • Styling is completely wrong. When resizing after the WebGL loads the website goes crazy.

This change is Reviewable

@CelineBoudier
Copy link
Contributor

the tests fail

@OlafSzmidt
Copy link
Contributor Author

Review status: 23 of 32 files reviewed, 7 unresolved discussions (waiting on @mrniket, @riaJha97, @OlafSzmidt, and @CelineBoudier)


players/tests.py, line 321 at r10 (raw file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

I guess it can cause confusion between two meanings: avatar_ID and game_ID. I'll try to change it back.

Done.


Comments from Reviewable

@OlafSzmidt
Copy link
Contributor Author

Review status: 23 of 32 files reviewed, 7 unresolved discussions (waiting on @mrniket, @riaJha97, @OlafSzmidt, and @CelineBoudier)


game_frontend/src/redux/features/Game/epics.test.js, line at r10 (raw file):

Previously, mrniket (Niket Shah) wrote…

I see your point. I guess the better way to test this would we should write tests for the api/unity folder. That way you could mock the SendMessage unity call and check that it returns the "fail" action

Don't think we can mock SendMessage() either. It's used here, look:

https://github.com/ocadotechnology/aimmo-unity/blob/b9eaba7a46f7cd57f3e03a8d2a4972747298820c/UnityProject/Assets/Scripts/UnitySocketIO/SocketIOWrapper.jslib

Instead, maybe we should handle errors on the Unity side and propagate them back to React somehow. No idea how.


Comments from Reviewable

@mrniket
Copy link
Contributor

mrniket commented Jun 22, 2018

Reviewed 9 of 9 files at r14.
Review status: all files reviewed, 1 unresolved discussion (waiting on @mrniket)


game_frontend/djangoBundler.js, line 15 at r13 (raw file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

You mean === if it's false we're expecting locally right (in non prod environments). If yes, done.

ah sorry, yes :)


game_frontend/src/redux/features/Game/epics.test.js, line at r10 (raw file):

https://github.com/ocadotechnology/aimmo-unity/blob/b9eaba7a46f7cd57f3e03a8d2a4972747298820c/UnityProject/Assets/Scripts/UnitySocketIO/SocketIOWrapper.jslib

Quick aside: that's the wrapper we plan to remove (in a separate issue) when we start listening for websocket events through the frontend instead of through unity, right?

Sorry, I referred to the wrong thing. I was talking about emitToUnity in api/unity.js as the method that we could perhaps mock for api/unity.js tests


Comments from Reviewable

@CelineBoudier
Copy link
Contributor

Reviewed 1 of 10 files at r8, 1 of 11 files at r13.
Review status: all files reviewed, 1 unresolved discussion (waiting on @OlafSzmidt)


players/utilities.py, line 1 at r10 (raw file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

So I'm happy to just resolve this if you are

helpers just sounds the same as utilities to me :/ I'd go with Niket's suggestion tbh


Comments from Reviewable

@CelineBoudier
Copy link
Contributor

:lgtm:


Review status: all files reviewed, 1 unresolved discussion (waiting on @OlafSzmidt)


Comments from Reviewable

@mrniket
Copy link
Contributor

mrniket commented Jun 25, 2018

:lgtm:


Review status: all files reviewed, 1 unresolved discussion (waiting on @OlafSzmidt)


Comments from Reviewable

@OlafSzmidt
Copy link
Contributor Author

Review status: all files reviewed, 1 unresolved discussion (waiting on @OlafSzmidt)


players/utilities.py, line 1 at r10 (raw file):

Previously, CelineBoudier (Celine Boudier) wrote…

helpers just sounds the same as utilities to me :/ I'd go with Niket's suggestion tbh

Done.


Comments from Reviewable

@CelineBoudier
Copy link
Contributor

:lgtm:


Review status: 30 of 32 files reviewed, 1 unresolved discussion (waiting on @CelineBoudier, @mrniket, @riaJha97, and @OlafSzmidt)


Comments from Reviewable

@CelineBoudier
Copy link
Contributor

Reviewed 1 of 2 files at r15.
Review status: 31 of 32 files reviewed, 1 unresolved discussion (waiting on @mrniket and @OlafSzmidt)


Comments from Reviewable

@CelineBoudier
Copy link
Contributor

Reviewed 1 of 2 files at r15.
Review status: all files reviewed, 1 unresolved discussion (waiting on @OlafSzmidt)


Comments from Reviewable

@mrniket
Copy link
Contributor

mrniket commented Jun 27, 2018

Reviewed 1 of 2 files at r15, 1 of 34 files at r19.
Review status: 1 of 36 files reviewed, all discussions resolved (waiting on @CelineBoudier, @mrniket, @riaJha97, and @OlafSzmidt)


Comments from Reviewable

@mrniket
Copy link
Contributor

mrniket commented Jun 27, 2018

Reviewed 1 of 36 files at r17, 1 of 1 files at r18, 33 of 34 files at r19.
Review status: :shipit: complete! all files reviewed, all discussions resolved


Comments from Reviewable

@mrniket mrniket merged commit dd2df80 into UI_new_design Jun 27, 2018
@mrniket mrniket deleted the add_unity_to_react branch June 27, 2018 13:23
OlafSzmidt pushed a commit that referenced this pull request Jul 4, 2018
* New UI: Move the UI contents into a new branch until fully functional (#547)

* setup code responses for react

* Add Editor Redux feature

* code review changes

* link editor code with django

right now the game id is fixed to 1

* test get code button

* Add GamePage snapshot test

* add post code epic

* Sends post request successfully

* add a way to get csrf token via ajax

* update npm packages

* add post request for code

* simplify passing of epics

* move api calls into an api folder

* simplify post api call

* simplify syntax

* Fix tests and add some more

* New UI: Code editor frontend (#548)

* setup code responses for react

* Add Editor Redux feature

* code review changes

* link editor code with django

right now the game id is fixed to 1

* Merge branch 'master' into django_editor_link

* test get code button

* Add GamePage snapshot test

* add post code epic

* Sends post request successfully

* add a way to get csrf token via ajax

* update npm packages

* add post request for code

* simplify passing of epics

* move api calls into an api folder

* simplify post api call

* simplify syntax

* Fix tests and add some more

* Add front-end component for AceEditor

* Modified theme of code editor

* Merge branch 'UI_new_design' into code_editor_frontend

* Fixed failing test and updated snapshot for code editor

* Merge branch 'code_editor_frontend' of https://github.com/ocadotechnology/aimmo into code_editor_frontend

* Editor state management (#555)

* Get intial state of code on editor load

* Added more types to handle changing code

* Wrote a test case for changeCodeEpic

* fix deps

* Inject testscheduler into changeCodeEpic

* State in post request should be evaluated at the time of the action

* fix eslint in GamePage tests

* code review changes

* Add Material Design to AI:MMO with a Basic Colour Palette (#618)

* Add material-ui core

* Add mui theme to MUIThemeProvider and styled components ThemeProvider

* Use theme colors for existing ui

I didn't do the Unity one because it's going to not have a background

* delete unused file

* add a withTheme testHelper

* Use withTheme testHelper

* Update snapshots

* Use absolute imports for testhelpers

* Merge branch 'master' into UI_new_design

* Fix build

* Add typography (#620)

* Add typography

* withTheme is a default export

* Fix tests so that they are testing the correct code

* Merge branch 'master'

Conflicts:
	game_frontend/package-lock.json
	game_frontend/package.json
	game_frontend/yarn.lock
	players/views.py

* JS: Update dependencies and lock files

* Add more snapshot tests for containers

* Merge with UI_new_design

* Document createShallowWithTheme (#641)

* Add documentation for `createShallowWithTheme` test helper

* Remove feature path

* Fix redux-dev-tools deployment bug

* [New UI] Unity Build in React (#616)

* Add react-unity-webgl package as a dependency

* Introduce new unity module to the component

* Update the README

* Move GameView to be a container

* Add some extra functionality local to the file for now

* Add utilities and a API with JSON settings. Resolves #612

* More functionality to connect two systems

* More Redux changes for success & connection

* Refactor method name of the view to not include 'view'

* Add initial state & send information across to Unity WebGL

* Add socketIO dependency to generated HTML

* Call Unity functions as redux actions now

* Remove irrelevant comments now

* Serialise boolean SSL flag for Unity

* More changes to how communication is conducted

* Handle errors

* Add lunch json for VS code to git ignore

* Emit success event in the epic

* Remove middlewares

* New feature introduced for Game

* Remove old game reducer and move to its own. Resolves #619

Same as below

* Separate epics for each event refactoring

* Remove older epic

* Update .gitignore launch json path
Ignore all .vscode files


Update .gitignore

* Remove a TODO comment

* Fix existing tests to suit new code

* Merge branch 'UI_new_design' into add_unity_to_react

* Regenerate yarn.lock

* Revert the refactoring, can be done in a different PR

* Add epic tests for the happy path

* Remove unwanted comment and add EOF lines

* Change reducer name, forgot to change beforehand

* Add reducer test for all current cases

* More function tests

* Some testing changes as requested in PR by @mrniket

* Change imports to one liner using aliases

* Remove redundant alias for default import

* Refactoring of setGameURL successful, rest not done

* Refactoring of setGameURL fixed

* Finish refactoring

* Merge remote-tracking branch 'origin/UI_new_design' into add_unity_to_react

* GameView minor esLint changes

* Merge remote-tracking branch 'origin/UI_new_design' into add_unity_to_react

* Fix indentation and other eslint issues in unity

* Certain eslint fixes

* More eslint

* Fix minor SSL bug

* Add extra developer settings to the parcel bundler

* Initial connectionParams naming changes


More naming changes


More naming changes again


connectionParams naming change done

* Name change in the view to parameters

* utilities called game_helpers now

* Add another shallow theme test to the layout

* Test passing, still unhappy with marble completion

* Use action creators instead of explicit objects

* Add css to Game Page testing

* Change the djangoBundler check environment before setting vars

* Update tests without theme to use shallow instead

* Export unity as a object containing all the calls instead

* Bring id of current avatar view to be called game_id again

* Add naming change to game_helpers as requested

* Fix tests to be correct now

* Make aimmo compatible with Django 1.9 (#642)

* Make aimmo compatible with Django 1.9

* Using patch version of Django 1.9 (#643)

* Using patch version of Django 1.9

* Merge remote-tracking branch 'origin/master' into add_unity_to_react

* Simply the API for api.js, put error in payload of fail actions

* Add test for emit unity failure

* Merge branch 'master' into UI_new_design
OlafSzmidt pushed a commit that referenced this pull request Jul 4, 2018
* New UI: Move the UI contents into a new branch until fully functional (#547)

* setup code responses for react

* Add Editor Redux feature

* code review changes

* link editor code with django

right now the game id is fixed to 1

* test get code button

* Add GamePage snapshot test

* add post code epic

* Sends post request successfully

* add a way to get csrf token via ajax

* update npm packages

* add post request for code

* simplify passing of epics

* move api calls into an api folder

* simplify post api call

* simplify syntax

* Fix tests and add some more

* New UI: Code editor frontend (#548)

* setup code responses for react

* Add Editor Redux feature

* code review changes

* link editor code with django

right now the game id is fixed to 1

* Merge branch 'master' into django_editor_link

* test get code button

* Add GamePage snapshot test

* add post code epic

* Sends post request successfully

* add a way to get csrf token via ajax

* update npm packages

* add post request for code

* simplify passing of epics

* move api calls into an api folder

* simplify post api call

* simplify syntax

* Fix tests and add some more

* Add front-end component for AceEditor

* Modified theme of code editor

* Merge branch 'UI_new_design' into code_editor_frontend

* Fixed failing test and updated snapshot for code editor

* Merge branch 'code_editor_frontend' of https://github.com/ocadotechnology/aimmo into code_editor_frontend

* Editor state management (#555)

* Get intial state of code on editor load

* Added more types to handle changing code

* Wrote a test case for changeCodeEpic

* fix deps

* Inject testscheduler into changeCodeEpic

* State in post request should be evaluated at the time of the action

* fix eslint in GamePage tests

* code review changes

* Add Material Design to AI:MMO with a Basic Colour Palette (#618)

* Add material-ui core

* Add mui theme to MUIThemeProvider and styled components ThemeProvider

* Use theme colors for existing ui

I didn't do the Unity one because it's going to not have a background

* delete unused file

* add a withTheme testHelper

* Use withTheme testHelper

* Update snapshots

* Use absolute imports for testhelpers

* Merge branch 'master' into UI_new_design

* Fix build

* Add typography (#620)

* Add typography

* withTheme is a default export

* Fix tests so that they are testing the correct code

* Merge branch 'master'

Conflicts:
	game_frontend/package-lock.json
	game_frontend/package.json
	game_frontend/yarn.lock
	players/views.py

* JS: Update dependencies and lock files

* Add more snapshot tests for containers

* Merge with UI_new_design

* Document createShallowWithTheme (#641)

* Add documentation for `createShallowWithTheme` test helper

* Remove feature path

* Fix redux-dev-tools deployment bug

* [New UI] Unity Build in React (#616)

* Add react-unity-webgl package as a dependency

* Introduce new unity module to the component

* Update the README

* Move GameView to be a container

* Add some extra functionality local to the file for now

* Add utilities and a API with JSON settings. Resolves #612

* More functionality to connect two systems

* More Redux changes for success & connection

* Refactor method name of the view to not include 'view'

* Add initial state & send information across to Unity WebGL

* Add socketIO dependency to generated HTML

* Call Unity functions as redux actions now

* Remove irrelevant comments now

* Serialise boolean SSL flag for Unity

* More changes to how communication is conducted

* Handle errors

* Add lunch json for VS code to git ignore

* Emit success event in the epic

* Remove middlewares

* New feature introduced for Game

* Remove old game reducer and move to its own. Resolves #619

Same as below

* Separate epics for each event refactoring

* Remove older epic

* Update .gitignore launch json path
Ignore all .vscode files


Update .gitignore

* Remove a TODO comment

* Fix existing tests to suit new code

* Merge branch 'UI_new_design' into add_unity_to_react

* Regenerate yarn.lock

* Revert the refactoring, can be done in a different PR

* Add epic tests for the happy path

* Remove unwanted comment and add EOF lines

* Change reducer name, forgot to change beforehand

* Add reducer test for all current cases

* More function tests

* Some testing changes as requested in PR by @mrniket

* Change imports to one liner using aliases

* Remove redundant alias for default import

* Refactoring of setGameURL successful, rest not done

* Refactoring of setGameURL fixed

* Finish refactoring

* Merge remote-tracking branch 'origin/UI_new_design' into add_unity_to_react

* GameView minor esLint changes

* Merge remote-tracking branch 'origin/UI_new_design' into add_unity_to_react

* Fix indentation and other eslint issues in unity

* Certain eslint fixes

* More eslint

* Fix minor SSL bug

* Add extra developer settings to the parcel bundler

* Initial connectionParams naming changes


More naming changes


More naming changes again


connectionParams naming change done

* Name change in the view to parameters

* utilities called game_helpers now

* Add another shallow theme test to the layout

* Test passing, still unhappy with marble completion

* Use action creators instead of explicit objects

* Add css to Game Page testing

* Change the djangoBundler check environment before setting vars

* Update tests without theme to use shallow instead

* Export unity as a object containing all the calls instead

* Bring id of current avatar view to be called game_id again

* Add naming change to game_helpers as requested

* Fix tests to be correct now

* Make aimmo compatible with Django 1.9 (#642)

* Make aimmo compatible with Django 1.9

* Using patch version of Django 1.9 (#643)

* Using patch version of Django 1.9

* Merge remote-tracking branch 'origin/master' into add_unity_to_react

* Simply the API for api.js, put error in payload of fail actions

* Add test for emit unity failure

* Merge branch 'master' into UI_new_design

* Fix linter error

* Use the new game_ide ui for AI:MMO and delete the old one 🙌

* Merge branch 'master' into UI_new_design

* Change travis.yml to build new UI before running all_tests


Silly mistake
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.

None yet

5 participants