Feature 155 #368
Feature 155 #368
Conversation
added automated loading of serializeable classes
| @@ -75,6 +75,7 @@ | |||
| import toniarts.openkeeper.tools.convert.map.Tile; | |||
| import toniarts.openkeeper.tools.convert.map.Variable; | |||
| import toniarts.openkeeper.utils.WorldUtils; | |||
| import toniarts.openkeeper.view.selection.SelectionArea; | |||
There was a problem hiding this comment.
We can't mix these like this. High hygiene levels must be maintained, especially during Covid season.
The logic needs to be separate from the view. If some common datatransfer etc. class is needed, it needs to be in another package. The VIEW basically uses logic classes yes, but definitely NOT the other way around.
There was a problem hiding this comment.
Perhaps the initial location of the SelectionArea class is not very good. It does not contain any visual component. All visual part contains in toniarts.openkeeper.view.selection.SelectionHandler
There was a problem hiding this comment.
I have tossed these common classes to toniarts.openkeeper.common, but also toniarts.openkeeper.game.data might be a place for this. Dunno, but not view if this is going to be used in logic.
And if this goes through network, as slim as possible. All you can generate or calculate, can be done so. Cached with transient etc fields.
There was a problem hiding this comment.
SelectionArea contains 2 Vector2f fields. Transmission surrendered a bit more expensive
There was a problem hiding this comment.
Well, if it is the same two fields that already went there, that is ok. This is from user interaction (meaning not all the time). So the extra layer is just few bytes.
There was a problem hiding this comment.
I generally think we need to use the Point class as often as possible. In most cases when working with a map, we work with tiles array indices, not vectors. SelectionArea also manipulate with indexes (not vectors)
There was a problem hiding this comment.
I agree using point instead of vectors. Vectors give me the chills. Also LOGIC shouldn't contain jME classes at all (or as little as possible at least). Or was this vector from the Math class or the libGdx... anyway.
| || room.getFlags().contains(Room.RoomFlag.PLACEABLE_ON_LAVA)) { | ||
| instancePlots = new ArrayList(mapController.getTerrainBatches(instancePlots, x1, x2, y1, y2)); | ||
| } | ||
| // if ((room.getFlags().contains(Room.RoomFlag.PLACEABLE_ON_WATER)) |
There was a problem hiding this comment.
Obsolete now? Shouldn't have code in comments, unless that is commented, otherwise very confusing later
There was a problem hiding this comment.
Must be deleted
| @@ -16,11 +16,13 @@ | |||
| */ | |||
| package toniarts.openkeeper.tools.convert.map; | |||
|
|
|||
| import com.jme3.network.serializing.serializers.EnumSerializer; | |||
There was a problem hiding this comment.
Yeah.. I guess this is ok for now, since it was already misused. But jME or network classes don't belong here. Same hygiene stuff.
|
Rooms are not yet transferred to the client. The reason being also that I was unsure what is the smartest way to do this. Currently it is just that raw map data as read only data on the client. Clients can parse these from the map data they have yes. But in addition to that, the clients need some information about the room usages and whatnots in the tooltip. Clients do also kinda have this information, but it needs a lot of processing. Like they know that when an entity is stored/using some room. Also they can calculate room capacities and all that.... One idea was that rooms are also entities. And the map has references to entityIDs. Server can preprocess the usage data to components. This would be really easy solution and allow the rooms to work fully via ES network structure. That also brings the idea of map tiles being entities as well. Updating and all that would also then be like with the others. |
|
o prevent possible cheats, we should not transmit all data about other players, if the current player does not see other players. In this regard, each client machine should not contain relevant information about rooms and tiles. Only Server should have all information |
There was a problem hiding this comment.
Copilot reviewed 73 out of 88 changed files in this pull request and generated no suggestions.
Files not reviewed (15)
- src/toniarts/openkeeper/game/component/CreatureImprisoned.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/ChickenAi.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/DoorViewState.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureAi.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/DoorComponent.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/AttackTarget.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureMood.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureMeleeAttack.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureFall.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureViewState.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/Death.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureSleep.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/ChickenGenerator.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/CreatureExperience.java: Evaluated as low risk
- src/toniarts/openkeeper/game/component/Decay.java: Evaluated as low risk
Need some help. Method
isSellable(SelectionArea ...in classPlayerInteractionState(methodgetColorIndicator()) not working. Problem with access in other thread. PropertyroomCoordinatesis empty in classMapController, but should not