search

AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.41k stars 288 forks source link

Extract flood fill algorithm from FillTilemapTool #612

Closed ChristianGreiner closed 6 years ago

ChristianGreiner commented 6 years ago

At the moment the flood fill alogrithm is implemented directly in the fill tilemap tool.

It would be clever if this alogrithm would be extracted, because you could reuse this code for other tilemap tools like a magic wand selection tool.

ilexp commented 6 years ago

Sounds good. Would be important to make sure that we don't introduce any performance downgrade by abstracting too much, since the flood fill is actually a bottleneck for very big maps. Do you have a design proposal for this?

Barsonax commented 6 years ago

Is it just me or does the flood fill method looks overly complex? Shouldnt it be a simple BFS algorithm?

EDIT: nvm seems its a clever algorithm, not very readable though. I do see some x,y to array index calculations that could be optimized away.

ChristianGreiner commented 6 years ago

Do you have a design proposal for this?

Maybe we could create a new class called FloodFillArea inside the Utility folder and move all methods and stuff from the FillTilemapTool into this class.

Any other suggestions?

ilexp commented 6 years ago

The Utility folder you linked is part of the editor plugin like the current place of the algorithm. I think this will be more useful if we find a core plugin location for the algorithm, similar to TilemapCulling. Maybe call the class TilemapFloodFill and put it next to it?

deanljohnson commented 6 years ago

This issue has been resolved, right? (https://github.com/AdamsLair/duality/pull/627)

ilexp commented 6 years ago

It has. Thanks! Closing this.