E finalize cbrshooter replayview

[leRoe93]

As discussed, here is the pull request that needs refactoring and adjustments to be MVVM conform.

Will be worked on by myself and @VocalTrance.

[mfroeh]

@FXML
public void handleFrameSlider() {
    viewModel.setSelectedFrame((int) frameSlider.getValue()).execute();
}

@FXML
public void handleVeloSlider() {
    viewModel.setUpdateInterval(100 / veloSlider.getValue()).execute();
}

@FXML
public void visualizeWeapon() {
    viewModel.visualizeMapElement("weapon", this.checkBoxWeapon.isSelected()).execute();
}

@FXML
public void visualizeAmmuItem() {
    viewModel.visualizeMapElement("ammuItem", this.checkBoxAmmuItem.isSelected()).execute();
}

@FXML
public void visualizeHealthItem() {
    viewModel.visualizeMapElement("healthItem", this.checkBoxHealthItem.isSelected()).execute();
}

These should probably all be handled by bindings instead. By calling methods with parameters from the view, we are creating dependency from viewmodel to view. We can see this here for example, since our viewModel methods expects the view to pass the exact string "ammuItem".

[mfroeh]

// Ensure that the pane has the same relative format as the game
paneSize.setX(550);

Get this from the rectangle instead.

// Update visibility of player icon
playerVisualsRow.getShowPlayerIconCheckBox().setOnAction(new EventHandler<ActionEvent>() {
    @Override
    public void handle(ActionEvent event) {

        var box = (CheckBox) event.getSource();
        var value = box.isSelected();

        mapElements.put(playerInfo.getName() + "_playerIcon", new Pair<Node, Boolean>(
                mapElements.get(playerInfo.getName() + "_playerIcon").getKey(), value));

        updateMapElements();
    }
});

We can bind the checkboxes selected property to our property and the listen to the changes on our property.

[mfroeh]

private void initializeVisualsTable() {
    ...
}

private void initializePlayerVisuals() {
   ...
}

By the name of it, should be a method of the view. We can set the data required for it in void initialize() in the viewModel.

[mfroeh]

private void updateMapElements() {
    ...
}

Ideally is a method in the view aswell, that is invoked whenever enough time passed that the frame should be forwarded.

In this case generally, the view model may not need to have more than a couple properties. Using these properties the exact frame and the speed at which frames should be shown is determined. Besides the properties, it has to expose the images to be used. Since the images do not change during the replay, we also don't have to use Image or ImageView but can just use the byte[] or string resource paths instead.

The ViewModel would drive what data is displayed and the view would determine in which way to display the data.

This is how I would do it anyway, because of not wanting to put very view dependant data, such as images or color, into the ViewModel. However, javafx clearly isnt a framework built with MVVM in mind. If I want to determine which data for charts I want to expose in the viewModel, I have to import import javafx.scene.chart.PieChart.Data; or import javafx.scene.chart.XYChart.Series; Any other approach would result in a solid 15+ additional lines, just to communicate if the data inside the observable list of my type has changed.

Therefore I believe that we can (and probably should) bend the rules a bit. I think having Images and colors inside the viewmodel is fine. ImageView sounds alot like a control, eventhought by package name it isnt, so maybe you can tell me exactly what this does. Methods returning commands should not have parameters but instead access bound properties. The methods initializing visuals should definetly be in the view.

We can take different approaches on the updating of visuals

1. Just move it all to the view and have a smaller viewmodel
2. Notify the view, that it should perform an update using mvvmfx publish-subscribe
3. Leave it all in the viewModel

Approach 1 think is the most MVVM like and will not cost alot of time to adjust to, 2 is also MVVM like but possibly confusing, 3 is already done ofcourse but not very MVVM like

[mfroeh]

We will take Leons approach, in which the viewmodel will termine the stats which will be shown and the view will do all visualization accordingly. The view knows when to visualize new data based on a ChangeListener to the viewModels selectedFrame property.

Note: Player needs properties (xD 3rd or 4th time swapping back and forth :smile:) so that the table can be updated without needing logic from us. We can simply use the PropertyValueFactory for that.