@ksiek127/rpg 84 display objects on screen #17

ksiek127 · 2022-04-21T16:10:24Z

Description

Checklist

Included code to test these changes
Updated Jira

methods private

Unit tests require these methods -> they must stay being visible inside a package

isOkValueNull, isErrorValueNull

# Conflicts: # src/main/java/io/rpg/Initializer.java # src/main/java/io/rpg/controller/Controller.java # src/main/java/io/rpg/model/location/LocationModel.java # src/main/java/io/rpg/view/GameObjectView.java # src/main/java/io/rpg/view/LocationView.java # src/main/java/io/rpg/viewmodel/LocationViewModel.java

kkafar

IMO generalnie git, zostawiłem kilka uwag. Możliwe, że będzie warto poczekać na mojego PRa bo planowałem tam zrobić kilka rzeczy o których napisałem w review -- chyba, że bym się ociągał (bo nie wiem na kiedy zrobię) to wtedy nie ma sensu czekać :D

Na pewno będzie trzeba wmergować tutaj mastera, bo zrobiłeś brancha odbijając się od mojej gałęzi, na której ja potem wiele plików usunąłem i kilka rzeczy zmieniłem -- a tych zmian już tutaj nie ma. Najłatwiej zrobić to wchodząc na swoją gałąź i mergując (wcześniej jeszcze warto pobrać aktualnego mastera):

git switch master
git pull
git switch @ksiek127/RPG-84-display-objects-on-screen
git merge master

Jak coś to mogę z tym pomóc.

kkafar · 2022-04-21T16:13:53Z

src/main/java/io/rpg/Initializer.java

+      GameObject gameObject = GameObject.fromConfig(goconfig);
+      gameObjects.add(gameObject);


Suggested change

GameObject gameObject = GameObject.fromConfig(goconfig);

gameObjects.add(gameObject);

gameObjects.add(GameObject.fromConfig(goconfig));

kkafar · 2022-04-21T16:18:15Z

src/main/java/io/rpg/view/IGameObjectModelStateChangeObserver.java

+package io.rpg.view;
+
+public class IGameObjectModelStateChangeObserver {
+}


Ten interfejs był ostatecznie wywalony. Zanim zmergujemy tego brancha będzie trzeba go wyczyścić z rzeczy, których już nie ma.

kkafar · 2022-04-21T16:18:28Z

src/main/java/io/rpg/view/ILocationModelStateChangeObserver.java

+package io.rpg.view;
+
+public interface ILocationModelStateChangeObserver {
+
+}


Podobnie tutaj.

kkafar · 2022-04-21T16:18:38Z

src/main/java/io/rpg/view/IObservable.java

+package io.rpg.view;
+
+public interface IObservable<T> {
+  void addListener(final T listener);
+
+  void removeListener(final T listener);
+}


kkafar · 2022-04-21T16:18:46Z

src/main/java/io/rpg/view/IOnClickedObserver.java

+package io.rpg.view;
+
+public interface IOnClickedObserver {
+  void onClick(GameObjectView source);
+}


kkafar · 2022-04-21T16:22:07Z

src/main/java/io/rpg/view/LocationView.java

    view.getViewModel().setBackground(new Image(config.getBackgroundPath()));
    // todo: na podstawie configu ustawić pola korzystając z view modelu
+    List<GameObjectConfig> objectConfigs = config.getObjects();
+    for(GameObjectConfig objectConfig: objectConfigs){


Suggested change

for(GameObjectConfig objectConfig: objectConfigs){

for(GameObjectConfig objectConfig : objectConfigs) {

kkafar · 2022-04-21T16:26:45Z

src/main/java/io/rpg/view/LocationView.java

+    List<GameObjectConfig> objectConfigs = config.getObjects();
+    for(GameObjectConfig objectConfig: objectConfigs){
+      GameObjectView goview = new GameObjectView(Path.of(objectConfig.getAssetPath()), objectConfig.getPosition());
+      viewModel.addChild(goview);
+    }


Jest spoko, ale można przemyśleć tworzenie obiektów GameObjectView już w Initializer. Wyciągnięcie tego tam i tak będzie potrzebne, zdaje się, bo będzie trzeba zarejestrować view do modelu (czyli każdy GameObjectView dodać jako observera do odpowiadającego mu GameObject). Prawdopodobnie zrobię w tym PR, ale jeszcze nie wiem na kiedy to będzie gotowe.

kkafar · 2022-04-21T16:30:44Z

src/main/java/io/rpg/viewmodel/GameObjectViewModel.java

+package io.rpg.viewmodel;
+
+import javafx.fxml.FXML;
+import javafx.fxml.Initializable;
+import javafx.scene.Parent;
+import javafx.scene.image.Image;
+import javafx.scene.image.ImageView;
+import javafx.scene.layout.HBox;
+import javafx.scene.layout.Pane;
+
+import java.net.URL;
+import java.util.ResourceBundle;
+
+public class GameObjectViewModel implements Initializable {
+    @FXML
+    private Pane imgPane;
+
+    @FXML
+    private ImageView gameObjectView;
+    @FXML
+    private HBox parent;
+
+    public GameObjectViewModel() {
+    }
+
+    public void setImage(Image image){
+        this.gameObjectView.imageProperty().setValue(image);
+    }
+
+    @Override
+    public void initialize(URL location, ResourceBundle resources) {
+        gameObjectView.imageProperty().addListener((property, oldImg, newImg) -> {
+            imgPane.setPrefWidth(newImg.getWidth());
+            imgPane.setPrefHeight(newImg.getHeight());
+        });
+    }
+}


Wydaje mi się, że te klasa nie będzie potrzebna, bo wszystko co potrzebujemy żeby narysować GameObjectView to ImageView, który jest jego superklasą -> chyba nie trzeba nic dodatkowego.

Edit: ViewModel był zrobiony dla klas LocationView i popupów bo one są ładowane z FMXL'a i JavaFX chce mieć takie coś (ona nazywa to kontrolerem (zresztą chyba słusznie, zależy jak patrzeć), ale my dostosowaliśmy nazewnictwo, żeby nie kolidowało z naszym kontrolerem). GameObjectView nie jest ładowane z FXML'a.

kkafar · 2022-04-21T16:31:32Z

src/main/java/io/rpg/viewmodel/LocationViewModel.java

+  public void addChild(ImageView child){
+    parent.getChildren().add(child);
+  }


Suggested change

public void addChild(ImageView child){

parent.getChildren().add(child);

}

public void addChild(ImageView child) {

parent.getChildren().add(child);

}

Dokładnie o to chodziło, gicik.

kkafar · 2022-04-21T16:32:28Z

src/main/resources/io/rpg/viewmodel/location-view.fxml

 <HBox fx:id="parent" alignment="CENTER" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="640.0" prefWidth="640.0" style="-fx-background-color: #cb2f2f;" xmlns="http://javafx.com/javafx/17" xmlns:fx="http://javafx.com/fxml/1" fx:controller="io.rpg.viewmodel.LocationViewModel">
   <children>
-      <Pane fx:id="contentPane" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="640.0" prefWidth="640.0">
+      <Pane fx:id="contentPane" viewOrder="1" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="640.0" prefWidth="640.0">


Z czystej ciekawośCi: co robi ten viewOrder="1"?

to jest taki jakby z-index, tzn kolejnosc, w ktorej sa wyswietlane rzeczy (np jesli sa dwa obiekty w tym samym miejscu to ten z wiekszym viewOrder zasloni ten z mniejszym)
teraz w sumie juz niepotrzebny, wyrzuce, to zostalo po tym, jak zrobilem fxml dla kazego gameobjectu i chcialem zeby byly rysowane na tle, a nie na odwrot

mhawryluk · 2022-04-24T00:31:41Z

src/main/java/io/rpg/viewmodel/LocationViewModel.java

  }
+
+  public void addChild(ImageView child){
+    parent.getChildren().add(child);


tak jak jest obecnie obrazki są rysowane obok tła, zamiast na nim. zmiana parent na contentPane chyba załatwia sprawę

Suggested change

parent.getChildren().add(child);

contentPane.getChildren().add(child);

mhawryluk · 2022-04-24T00:34:35Z

src/main/java/io/rpg/view/LocationView.java

    // todo: na podstawie configu ustawić pola korzystając z view modelu
+    List<GameObjectConfig> objectConfigs = config.getObjects();
+    for(GameObjectConfig objectConfig: objectConfigs){
+      GameObjectView goview = new GameObjectView(Path.of(objectConfig.getAssetPath()), objectConfig.getPosition());


pozycję w postaci numeru wiersza i kolumny trzeba jeszcze przeskalować przez rozmiar kwadratu siatki. raczej nie w tym miejscu, ale gdzieś trzeba

…creen

kkafar and others added 30 commits April 11, 2022 20:15

chore: Massive update...

f909673

BREAKING CHANGES: Arch proposal

97168b4

chore: rename io.rpg.torefract -> io.rpg.torefact

eb85cb5

refact: rename PointsPopupController -> PointsPopupViewModel

b5a0c8a

refact: add comments & change method names in ConfigLoader

b69f869

refact: add comments tgo GameObjectConfig

8eb9b59

refact: add comments to GameWorldConfig

25a08a5

chore: make ConfigLoader#{loadGameWorldConfig,loadLocationConfig}

de086eb

methods private

chore: add generic class describing operation result

a958a33

chore: add Optional returning getters to Result

f1219eb

fix: revert changes from #de086eb

211f928

Unit tests require these methods -> they must stay being visible inside a package

chore: update configuration files

7fe4bd6

chore: improve error handlign in config module via Result class

e774cfb

feat: add helper methods to Result class

04292a5

isOkValueNull, isErrorValueNull

chore: add builder to the Game class

77279ff

refact: adapt Main to new Result based interface

2223fa0

feat: add builder to the Controller

4a27c40

chore: update unit test configuration

41f013f

fix: make unit tests pass

a9d9c92

chore: Update observer schema before refractoring

f85525c

Merge branch 'master' into @kkafar/arch-proposal

58ed78f

docs: add more comments to GameWorldConfig

1af5380

chore: make GameObjectConfig#valide method return Result

8f82497

chore: Cleanup ConfigLoader

8af785a

chore: minor fixes & apply linter

9f65d92

refact: apply style suggestions

c900f2d

Merge branch 'master' into @kkafar/arch-proposal

52cc4f6

chore: attempt to display game objects on screen

448ab7e

chore: displaying objects on screen v1

662d640

ksiek127 requested a review from kkafar April 21, 2022 16:10

kkafar reviewed Apr 21, 2022

View reviewed changes

chore: displaying objects on screen v2

d60e29d

mhawryluk reviewed Apr 24, 2022

View reviewed changes

kkafar changed the base branch from master to development April 24, 2022 12:25

Merge branch 'development' into @ksiek127/RPG-84-display-objects-on-s…

ef4db38

…creen

kkafar merged commit 10f3f70 into development Apr 24, 2022

mhawryluk deleted the @ksiek127/RPG-84-display-objects-on-screen branch April 30, 2022 22:32

		GameObject gameObject = GameObject.fromConfig(goconfig);
		gameObjects.add(gameObject);

	GameObject gameObject = GameObject.fromConfig(goconfig);
	gameObjects.add(gameObject);
	gameObjects.add(GameObject.fromConfig(goconfig));

	for(GameObjectConfig objectConfig: objectConfigs){
	for(GameObjectConfig objectConfig : objectConfigs) {

	parent.getChildren().add(child);
	contentPane.getChildren().add(child);

@ksiek127/rpg 84 display objects on screen #17

@ksiek127/rpg 84 display objects on screen #17

Uh oh!

Conversation

ksiek127 commented Apr 21, 2022

Description

Checklist

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkafar Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kkafar Apr 21, 2022 •

edited

Loading