MVC doubts

[andreaalf97]

Good afternoon, this is the first time I'm working with MVC, Observers, Observables, and Threads, and I'm finding very conflicting content about it on the internet, so I'm opening this issue hoping to receive some feedback about our first lines of code on the Controller:

I understood that the core of the server side are the Model, the Controller and the Virtual View, which is the only one communicating with the Client View (one for each user, on the client side). The Controller governs the logic of the game and modifies the Model based on the information received by the Virtual View, so the relationship between these elements should be something like this: -The Controller is an Observer for the Virtual View -The Virtual View is an Observer of the Model

If this is correct, I'm having some troubles understanding how to make the Controller and the Virtual View communicate when, during a turn, the Controller needs some kind of user input to continue. This is what we have done so far: The Controller class extends Thread and runs the entire game inside the overridden run() method. It is also provided with a boolean dataIsReady; and an object used as a lock final Object lock;. What I'm doing whenever the Controller needs new user input is something like this:

    //This is inside the method public void run()

    this.dataIsReady = false;
    //Asking the view for the required data
    this.view.askForIndexPowerupToDiscard(currentPlayer, this.gameModel.getPlayerPowerups(currentPlayer));

    //waits until data is ready
    while(!this.dataIsReady) {
        try {
            sleep(1000);
        } catch (InterruptedException e) {
            Log.LOGGER.warning(e.getMessage());
        }
    }

    synchronized(lock){
        //Using the data that can now be found in (e.g.) this.view.data to continue the game
    }

The idea is that I will exit the while(!this.dataIsReady) only through the public void update(Observable o, Object arg) method, which looks like this:

    @Override
    public void update(Observable o, Object arg) {
        synchronized (this.lock){
            this.dataIsReady = true;
        }
    }

If what I understood is right, the update method is called by the view through

    setChanged();
    notifyObservers();

when the data is ready

Thank you, Andrea Alfieri

[ingconti]

1) Andrea Corsini will refocus on observer/model/controller logic, as taught at lesson. 2) In the following I will write fond come Caveats and DONT when dealing with O.S. concepts.

[ingconti]

"DONT" list:

before diving into observer / model / various message passing pls note: a) there is no reason to thread the controller. we do not need more controllers. so why thread them? (or are you thinking to use ONE controller for every player?)

b) I personally will thread after receiving connections.

c) we will have a thread for every client, that will call "atomic" / synchronised methods inside controller / model (here depends on your logic)

d) be very careful mixing locks, normal data member and sleep. a wrong mix of the usually brings the app to a lethal behaviour.

so for example: d1) avoid sleep. A thread is already automatically waiting if no input, so some methods will not be called until data from network. (i.e. key pressure)

You are implementing a polling strategy with sleep and exception, but is suboptimal.

d2) You will exit loop when

"this.dataIsReady = true;"

but be coherent:

You you use a lock to protect a bool, why not in every point you READ the value inside the loop? some producer / consumer schema??

for a simple bool as in your case, you can avoid locks:

even if you read while other thread is updating / writing, the only case is you miss a round... but on next cycle, it will be set.

locks:

I will instead protect every method (and better to centralise access in only ONE functions..) that can be called from multiple thread (i.e.) players:

example (taken from slides about unit test...already discussed ...)

Boolean login(String nick ) { allocateLazy(); int count = nicknames.size(); if (count >= 4) return false; nicknames.add(nick); return true; }

we must protect BOTH count AND add:

so. for examople:

Boolean login(String nick ) { synchronized (this.lock){ allocateLazy(); int count = nicknames.size(); if (count >= 4) return false; nicknames.add(nick); } return true; }

(removed lock mng. for semplicity)

Also logout...must be protected, as it does remove from list.

[andreaalf97]

Thank you, So I will remove the lock and the boolean value and just use calls like

int indexToDiscard = this.view.askForIndexPowerupToDiscard(currentPlayer, this.gameModel.getPlayerPowerups(currentPlayer));

which automatically waits for the data to arrive, as you said.

As for a), we are implementing the multiple game feature, and I was thinking about using one Controller for each game, that's why I thought about Threads

[ingconti]

"multiple game feature" you should have: 1) a server that listen and create thread for every client 2)the server will detach another GAME (non a controller..) and new clients will join it.

[keni7385]

I understood that the core of the server side are the Model, the Controller and the Virtual View, which is the only one communicating with the Client View (one for each user, on the client

Ok

The Controller governs the logic of the game and modifies the Model based on the information received by the Virtual View, so the relationship between these elements should be something like this: -The Controller is an Observer for the Virtual View -The Virtual View is an Observer of the Model

Right

For simplicity, firstly I suggest you to reason about the MVC architecture without consider the fact that your MVC is distributed (if you haven’t done it yet). Let’s imagine that all the users play from the same computer, same executable. In this case, you would have multiple views that the controller observes, one for each player. Every time a player interacts, the controller get notified about the user inputs, processes them and potentially updates model objects. If the model got updated, then the views would change as well (not because of the controller), since they observe the model.

At this stage, there are already some design choices to take. For example how to handle questions (e.g. which power up index) to single players? Asynchronously (tell the player there is something to choose, then potentially the controller will receive the input) or synchronously (your current solution)? Which are the pros and cons of both of them? Poll/pull or push strategy?

Once you figured out this (you can follow up here if you have already done it), then consider that you have different kind of view (cli, gui, remote ones). Once you choose be coherent, don’t mix approaches. If your design is good, you will be able to play a little MVC, even without network interfaces in the middle.

If this is correct, I'm having some troubles understanding how to make the Controller and the Virtual View communicate when, during a turn, the Controller needs some kind of user input to continue.

If you want to go for a blocking solution, the thread that will “wait” it’s the same one that handles the connection. If you are not considering the network, it’s simply a method invocation.

This is what we have done so far: The Controller class extends Thread and runs the entire game inside the overridden run()

As ingconti replied, I would avoid it too. Try to reason on the difference between threads (parallel execution flows) and objects/resources used by different thread. So a single controller can be accessed by the multiple remote views, whose code is running in the thread that is managing the client inputs.

For example (asynchronous way), you got the observer/observable pattern, so you can use it to receive the power up choice as well. Therefore you don’t need to lock threads in which you are executing the controller’s code. Simply, you can have two methods: one method that asks for the index, the second one with the code after your ‘while’. The latter will be called once the controller observes the user choice for the index.

[andreaalf97]

Thank you! We will work on this to have something to show on 04/30. I have one last doubt about last part of @keni7385 's answer: If we keep a single method inside the controller which runs the entire game it would look like this:

Log.LOGGER.info("Game starting");
Log.LOGGER.warning("The map has been chosen by polling");
Log.LOGGER.warning("The KST has been set up after polling");

//Runs the first turn for all players
for(String currentPlayer : gameModel.getPlayerNames()){

    //Spawn player by choosing 2 powerups!
    //At this point the player should have nothing in his hands
    gameModel.givePowerup(currentPlayer);
    gameModel.givePowerup(currentPlayer);

    //Ask player which one to discard
    //Respawn player

    //Do you want to use a powerup?

    //Move - Move&Grab - Attack

    //Do you want to use a powerup?

    //Move - Move&Grab - Attack

    //Do you want to use a powerup?

    //Do you want to reload?

    gameModel.checkDeaths();
    gameModel.refillAmmos();
    gameModel.refillSpawns();
}

//Runs until the end of the regular game, will handle frenzy in the next loop
while( /*There are still skulls on the kill shot track*/ ){

    for(String currentPlayer : gameModel.getPlayerNames()) {

        //Do you want to use a powerup?

        //Move - Move&Grab - Attack

        //Do you want to use a powerup?

        //Move - Move&Grab - Attack

        //Do you want to use a powerup?

        //Do you want to reload?

        gameModel.checkDeaths();
        gameModel.refillAmmos();
        gameModel.refillSpawns();
    }
}

for(String currentPlayer : gameModel.getPlayerNames()){
    //TODO check if the round continues from the last player
    gameModel.setupForFrenzy();
}

//Run frenzy turn for each player

gameModel.giveKSTpoints();
gameModel.endGame();

Where all methods that modify the Model are synchronized and where commuication with each player would work like this (i.e. when respawning a player):

int chosenPowerupToDiscard = this.view.askForIndexPowerupToDiscard(currentPlayer, this.gameModel.getPlayerPowerups(currentPlayer));;

gameModel.respawn(currentPlayer, chosenPowerupToDiscard);

If we proceed in this direction, we don't need the Controller to be observing the view, since the communication happens only through methods invocations. Only the View would be observing the Model for changes. Would this be a valid solution?

[keni7385]

I have many doubts about this approach. Firstly, how do you tackle players that leave the game? For instance we have 5 players, 1 leaves... how can you inform the other players? Or even worst, how can you stop the game (in case the number of left players is < 2)? All the game lives inside a method, I feel it's much more difficult to manages simultaneous events (e.g. logins or client failures, that are the only simultaneous events, since the game per se is strictly sequential). Secondly, what about clients? The problem arises especially for the GUI one, that is intrinsically a multi-threaded client. The experience would look like a CLI emulated on the graphical user interface. It's true that the UX is not our main goal, but a non-blocking client (especially the GUI) is warmly preferred.

If we proceed in this direction, we don't need the Controller to be observing the view, since the communication happens only through methods invocations.

You will always have a method invocation, for instance:

View:
for obs in observers {
    obs.update(new MoveAction(x, y);
    or obs.updateMoveAction(x, y);
}

I cannot see advantages in this approach, over a stateless controller (the state is in the model). In this case, the user act actively, everything start from there. Then the controller can start a game, modify the model, activate a weapon, ... based on the inputs that observes from the view. Changes in the view, new opportunity for actions should arrive from the model.

Please, list some advantages of your approach here.

[ingconti]

I agree with Corsini

On 26 Apr 2019, at 10:02, Andrea Corsini notifications@github.com wrote:

I have many doubts about this approach. Firstly, how do you tackle players that leave the game? For instance we have 5 players, 1 leaves... how can you inform the other players? Or even worst, how can you stop the game (in case the number of left players is < 2)? All the game lives inside a method, I feel it's much more difficult to manages simultaneous events (e.g. logins or client failures, that are the only simultaneous events, since the game per se is strictly sequential). Secondly, what about clients? The problem arises especially for the GUI one, that is intrinsically a multi-threaded client. The experience would look like a CLI emulated on the graphical user interface. It's true that the UX is not our main goal, but a non-blocking client (especially the GUI) is warmly preferred.

If we proceed in this direction, we don't need the Controller to be observing the view, since the communication happens only through methods invocations.

You will always have a method invocation, for instance:

View: for obs in observers { obs.update(new MoveAction(x, y); or obs.updateMoveAction(x, y); } I cannot see advantages in this approach, over a stateless controller (the state is in the model). In this case, the user act actively, everything start from there. Then the controller can start a game, modify the model, activate a weapon, ... based on the inputs that observes from the view. Changes in the view, new opportunity for actions should arrive from the model.

Please, list some advantages of your approach here.

[andreaalf97]

We thought about this approach because the game is just the same sequence of events, repeated over and over until there are no more skulls on the KST. But I understand how the other approach might be a better solution.

So if I understand it correctly, the Controller is just waiting for any kind of user input by any player (coming through the View): every time new input is ready (the update funct is called), the Controller needs to check if it comes from the right player, at right time, and is a valid input (for instance a player who wants to move 10 spots away would not always be accepted). But to be able to do this, the Controller needs some sort of state, or at least he should know whose turn it is. And the Model now needs a list of possible actions for each player in the game, which is communicated to the players through the View (which observes the Model)

[keni7385]

We thought about this approach because the game is just the same sequence of events, repeated over and over until there are no more skulls on the KST. But I understand how the other approach might be a better solution.

So if I understand it correctly, the Controller is...

Right

And the Model now needs a list of possible actions for each player in the game, which is communicated to the players through the View (which observes the Model)

If you go for this way, make sure that your model can support this route