Integrate socket.io connection with React

[mossjacob]

Completes #675, #634, #635

---

This change is 

[CelineBoudier]

Reviewed 3 of 8 files at r1, 8 of 12 files at r2.
Reviewable status: 11 of 15 files reviewed, 3 unresolved discussions (waiting on @JCobbles)

---

aimmo-game/service.py, line 25 at r2 (raw file):

app = flask.Flask(__name__)
CORS(app, supports_credentials=True)
sio = socketio.Server()

if we want to keep the socketio name we could use import socketio as something else.
I'm just wondering if sio is a good name

---

aimmo-game/service.py, line 124 at r2 (raw file):

if __name__ == '__main__':
    logging.basicConfig(level=logging.INFO)
    host, port = sys.argv[1], int(sys.argv[2])

can't it fail to get enough args?

---

game_frontend/src/containers/GameView/index.js, line 19 at r2 (raw file):

    RegisterExternalListener('SendAllConnect', this.sendAllConnect.bind(this))
    // RegisterExternalListener('ClientReady', this.clientReady.bind(this))

?

[NiallEgan]

Reviewable status: 10 of 15 files reviewed, 4 unresolved discussions (waiting on @CelineBoudier and @JCobbles)

---

aimmo_runner/runner.py, line 44 at r3 (raw file):

        os.environ.setdefault("DJANGO_SETTINGS_MODULE", "example_project.settings")

    django.setup()

Why has this been added? This should be automatically called when we use manage.py.

[mossjacob]

Reviewable status: 8 of 17 files reviewed, 4 unresolved discussions (waiting on @CelineBoudier, @JCobbles, and @NiallEgan)

---

aimmo-game/service.py, line 25 at r2 (raw file):

Previously, CelineBoudier (Celine Boudier) wrote…

if we want to keep the socketio name we could use import socketio as something else.
I'm just wondering if sio is a good name

Done.

---

aimmo-game/service.py, line 124 at r2 (raw file):

Previously, CelineBoudier (Celine Boudier) wrote…

can't it fail to get enough args?

this is not really a new addition - it would fail without enough args before these changes. I would assume the arguments are always passed into the game, it wouldn't be able to function without it either way.

---

aimmo_runner/runner.py, line 44 at r3 (raw file):

Previously, NiallEgan (Niall Egan) wrote…

Why has this been added? This should be automatically called when we use manage.py.

Niket added this - I'm not sure why it is here but I'll remove it for now and ask him when he returns on monday

[mossjacob]

Reviewable status: 8 of 17 files reviewed, 4 unresolved discussions (waiting on @CelineBoudier and @NiallEgan)

---

game_frontend/src/containers/GameView/index.js, line 19 at r2 (raw file):

Previously, CelineBoudier (Celine Boudier) wrote…

?

Done.

[CelineBoudier]

Reviewed 2 of 12 files at r2, 1 of 2 files at r3, 8 of 8 files at r4.
Reviewable status: 14 of 16 files reviewed, 1 unresolved discussion (waiting on @NiallEgan)

---

aimmo-game/service.py, line 124 at r2 (raw file):

Previously, JCobbles (Jacob Moss) wrote…

this is not really a new addition - it would fail without enough args before these changes. I would assume the arguments are always passed into the game, it wouldn't be able to function without it either way.

true. just looked awkward either way

[CelineBoudier]

Reviewed 1 of 12 files at r2, 1 of 2 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @NiallEgan)

[NiallEgan]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @NiallEgan)

---

aimmo-game/service.py, line 124 at r2 (raw file):

Previously, CelineBoudier (Celine Boudier) wrote…

true. just looked awkward either way

Also this is a problem in many parts of the code, maybe would be good to rectify in a separate PR?

[NiallEgan]

Reviewable status: complete! all files reviewed, all discussions resolved

[NiallEgan]

Reviewed 2 of 8 files at r1, 5 of 12 files at r2, 2 of 2 files at r3, 8 of 8 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

[CelineBoudier]

Reviewable status: complete! all files reviewed, all discussions resolved

---

aimmo-game/service.py, line 124 at r2 (raw file):

Previously, NiallEgan (Niall Egan) wrote…

Also this is a problem in many parts of the code, maybe would be good to rectify in a separate PR?

+1

[CelineBoudier]

Reviewable status: complete! all files reviewed, all discussions resolved

[mossjacob]

Reviewed 1 of 1 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved

[OlafSzmidt]

Reviewed 4 of 12 files at r13, 4 of 4 files at r14.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @JCobbles and @NiallEgan)

---

aimmo-game/service.py, line 128 at r8 (raw file):

Previously, mrniket (Niket Shah) wrote…

Not quite sure how you want it the debug flag to be set from your description, could you give an example of usage?

Dockerfile has this line in its config:

CMD ["python", "./service.py", "0.0.0.0", "5000"]

We can have a check here based on ENV vars to see if we're developing or on production. This goes to service.py, if name is main, which it is, we can call run_game with this flag, and pass it all the way down to the debug flag of eventlet, plus in the future do some setting logging levels for example?

Just food for thought

---

aimmo_runner/runner.py, line 49 at r10 (raw file):

Previously, mrniket (Niket Shah) wrote…

what was the resolution for this @OlafSzmidt @NiallEgan ?

Nothing yet (?)

[OlafSzmidt]

Reviewed 4 of 12 files at r13.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mrniket and @NiallEgan)

[mrniket]

Reviewable status: 17 of 18 files reviewed, 2 unresolved discussions (waiting on @OlafSzmidt, @JCobbles, and @NiallEgan)

---

aimmo-game/service.py, line 128 at r8 (raw file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

Dockerfile has this line in its config:

CMD ["python", "./service.py", "0.0.0.0", "5000"]

We can have a check here based on ENV vars to see if we're developing or on production. This goes to service.py, if name is main, which it is, we can call run_game with this flag, and pass it all the way down to the debug flag of eventlet, plus in the future do some setting logging levels for example?

Just food for thought

It sounds like this is ⚠️ out of scope for this task, since it was debug=False before with the old socket library. Adding an ability to change it sounds like an extra feature.

It sounds like a good idea for investigation, maybe we can create an issue for it?

[mrniket]

Reviewable status: 17 of 18 files reviewed, 2 unresolved discussions (waiting on @OlafSzmidt and @NiallEgan)

---

aimmo_runner/runner.py, line 49 at r10 (raw file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

Nothing yet (?)

From slack it looks like we want to keep it, @NiallEgan can you resolve this discussion please?

[mrniket]

Reviewed 1 of 1 files at r15.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @OlafSzmidt and @NiallEgan)

[NiallEgan]

Reviewed 1 of 6 files at r8, 7 of 12 files at r13, 4 of 4 files at r14, 1 of 1 files at r15, 1 of 1 files at r16.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @OlafSzmidt)

[OlafSzmidt]

Reviewable status: complete! all files reviewed, all discussions resolved

---

aimmo-game/service.py, line 128 at r8 (raw file):

Previously, mrniket (Niket Shah) wrote…

It sounds like this is ⚠️ out of scope for this task, since it was debug=False before with the old socket library. Adding an ability to change it sounds like an extra feature.

It sounds like a good idea for investigation, maybe we can create an issue for it?

Sure.

[OlafSzmidt]

Reviewable status: complete! all files reviewed, all discussions resolved

[mrniket]

Reviewable status: complete! all files reviewed, all discussions resolved

[mrniket]

Reviewable status: complete! all files reviewed, all discussions resolved

[mrniket]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JCobbles)

a discussion (no related file):
block until @jcobbles resolves the issues he is having

---

[mossjacob]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mrniket)

a discussion (no related file):

Previously, mrniket (Niket Shah) wrote…

block until @jcobbles resolves the issues he is having

Done.

---

[mrniket]

Reviewable status: complete! all files reviewed, all discussions resolved

[mrniket]

Reviewed 1 of 1 files at r17.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JCobbles)

---

game_frontend/src/containers/GameView/index.js, line 14 at r17 (raw file):

export class GameView extends Component {
  onProgress (progression) {
    if (progression === 1) {

maybe we should make this into a constant?

[NiallEgan]

Reviewed 1 of 1 files at r17.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JCobbles)

[mossjacob]

Reviewable status: 17 of 18 files reviewed, 1 unresolved discussion (waiting on @NiallEgan and @mrniket)

---

game_frontend/src/containers/GameView/index.js, line 14 at r17 (raw file):

Previously, mrniket (Niket Shah) wrote…

maybe we should make this into a constant?

Done.

[mrniket]

Reviewed 1 of 1 files at r18.
Reviewable status: complete! all files reviewed, all discussions resolved

[mossjacob]

Reviewable status: complete! all files reviewed, all discussions resolved

[mossjacob]

Reviewed 2 of 2 files at r19.
Reviewable status: complete! all files reviewed, all discussions resolved

[mossjacob]

Reviewed 1 of 6 files at r8, 1 of 12 files at r13, 4 of 4 files at r14, 1 of 1 files at r15, 1 of 1 files at r16, 1 of 1 files at r18.
Reviewable status: complete! all files reviewed, all discussions resolved