Implement Map Decoration Objects

[tomsoftware]

I create a sub branch (of feature_original_maps https://github.com/tomsoftware/settlers-remake/tree/add_decorations_objects) to implement map-decoration-objects. May you can create a branch in the remake so anyone can help? Or may I Push this in "feature_original_maps" ?

Current State (Working):

ToDo:

• Some Fixes in the "MapObjectDrawer"
• "mapcreator"
• blocking area of objects

Thanks.

[codingberlin]

Please PR this into the enhancement_load_original_maps branch.

[tomsoftware]

OK, I will do this, but because it is not a Original-Map-loader issue, I first will try to finish my clean up/refactor of the MapObjectDrawer Class... not sure if this is a good idea but it looks mutch smarter to me if the IMapObjects "know" how to draw them and not the Drawer it self. We will see.

[codingberlin]

For this question @michaelzangl is the better person of contact.

[andreas-eberle]

@tomsoftware: The problem here is that the IMapObjects are plain "game logic"-objects. Therefore they have no idea of the UI / rendering. The idea is to keep game logic and UI completely separated (although this is already broken for some interfaces, rendering is certainly part of the graphics projects).

Regarding the PR: if your PR only contains the rendering for decoration objects / no changes regarding the reading of orignal maps, I think you could also PR it against the master directly, as it does not depend on the original map reading.

[tomsoftware]

@andreas-eberle : Yes, you are right! Separating game and graphics is very important!

But I have some problems with the current design:

• I need a additional style flag for Trees and decoration cause e.g. "Palm trees" should only turn up in deserts (so don't use {x,y} style) and decoration objects have different styles but not a different behaviour in Game logic.
• Currently you are using loosely constants in MapObjectDrawer to define sequences and frames for e.g. trees... I need mach of them.

So the game logic needs to know:

• State and Type of object (e.g. TREE_GROWING, TREE_ADULT, TREE_DEAD, AND progress)

And the graphic logic needs an additional info:

• STYLE of e.g. Tree / Deco-Object / Building

So the IMapObjects supplies the Logic part and I think it can also supply an additional abstract "Stylestate" value.

Also you are doing this with building.getBuildingType().getImages(); for the Buildings... why not for all Objects?

So please have a look at https://github.com/tomsoftware/settlers-remake/blob/add_decorations_objects/jsettlers.graphics/src/jsettlers/graphics/map/draw/MapObjectDrawer.java#L140 and tell me if this is a good or bad idea :-)

@michaelzangl : thanks for your comments... I add them

[tomsoftware]

OK... slowly I understand. You try to separate and cover all logic and graphics functionality from the object itself. But Why?

You have a 3 things:

• the object (=> IMapObject)
• the MapObjectsManager (with logic functions for the object)
• the MapObjectDrawer (with drawing functions for the object)

The way you are doing this is (I think) confusing and cause problems:

• Objects already have some logic parts like Tree.GROWTH_DURATION or AdultTree.getGrowthDuration()
• MapObjectsManager and MapObjectDrawer are crowded with functions
• It is not possible to call plantTree(ShortPoint2D pos, ETreeTypes treeType) from outside MapObjectsManager (I need this from the OriginalMapReader to create new, growing Trees)

Solution:

• move it's logic to the object (=> IMapObject)
• MapObjectsManager only calls the logic functions of the object but don't know about the object-logic
• MapObjectDrawer only draw as the object like to be drawn

class Tree {
  public plantNewTree() {
     schedule(tree, Tree.GROWTH_DURATION, false);
  }
  public getDrawStyle() {
    return style
  }
}

class MapObjectsManager {
  private plantTree() {
     Tree tree = new Tree(pos, treeType);
     tree.plantNewTree();
  }
}

class MapObjectDrawer  {
  private drawTree {
    style = tree.getDrawStyle();
    draw_link_style_says(style);
  }
}

[andreas-eberle]

@tomsoftware: You are right about "this is confusing". The system grew a lot and was never really restructured to better fit the needs. Therefore thanks for bringing up this topic.

First of all: why we tried to decouple the MapObjects from both, game logic and graphics:

• Storing only game relevant information is important to keep memory consumption lower and the savegame clean of non-deterministic stuff. E.g. it would be a bad idea to store the current image index of an adult tree that is used for animating it. This is only a UI thing and does not have any effect on the game logic. However, as we cannot store information in the graphics system but only calculate it on demand (like for the tree animation), we have to store some information in the MapObject. For example the type of the tree needs to be stored, although the game logic only cares about "is there an obstacle on the map" and "can this tree be cut".
• Some map objects need to progress over time (like growing trees). Furthermore, some objects also need to interact with the rest of the game (e.g. a fallen tree or a resource sign dissappear after a while). Giving all these objects a reference to the MainGrid (or an interface for there interaction with the MainGrid) would really increase memory usage (to amount killing the game on Android). I know, optimization should be the last part of development, but it also needs to be kept in mind when choosing solutions. That's why the MapObjectsManager was introduced. It manages the objects by executing their logic, especially the logic interacting with the rest of the game.

So far regarding the ideas behind the system. Of course, this is far from an ideal solution and I'm open for a rework. However, some things need to be considered for such a rework:

• Refactorings like this should not change the behavior. Therefore I would like to have this done in a separate branch without the combined integration of decoration objects. Mixing a major refactoring with a big new feature only makes it more complicated. Of course, the needs of the decoration objects must be kept in mind.
• The graphics and game logic should be as separated as possible.
• Memory consumption and garbage creating is always an issue on the Android platform and must be kept in mind.

After taking a look at you code in the "add_decoration_objects"-branch, I think your DrawableObjectFrame is a way to start. We really should try to find a unified way to render map objects.

Regarding the proposal for the MapObjectsManager in your last comment: I think this will not work due to the reasons why we introduced the MapObjectsManager in the first time. I think it will be better to search a unified way of handling the MapObjects in the MapObjectsManager, like you already did for the MapObjectDrawer.

All in all, thanks for bringing this to attention and thanks for your idea on unifying the MapObject drawing. I think we should move this topic to a separate issue and continue discussion there. For the development, I will open a new and clean branch in the main project, where we can work together.

Is this ok with you @tomsoftware?