Dijkstra_map_for_Godot
Dijkstra_map_for_Godot copied to clipboard
Remote Feature
Hi!!
I refactored the code around a macro and operation on the map.
This works and pass all the test and is more expressive but more complex (this was one of my first macro : a pain) although some of the new code could be improved, I'm also unsure about the naming.
If you're interested in giving me a review you're more than welcome ! @arnaudgolfouse @MatejSloboda
Also note that you cannot undo a remove connection yet, it should be easy to implement.
Also note that Documentation/Comments surrounding this PR could be improved
The method generated by the macro return a Result, the impl to return this result is correct but stupid, I doubt the use for Errors is really useful and this probably could be improved as well, open to suggestions.
The more I look at it, the more I wonder if the macro enum_operation
is truly necessary. Could we not just use DijkstraMap::apply_operation
everywhere ?
Maybe then you'll say that it will be hard keeping Remote
and DijkstraMap
in sync, but consider this : if we only use apply_operation
, we only need one method on Remote
:
impl Remote {
pub fn apply_operation(&mut self, operation: Operation) { // bikeshiddable name, of course
self.operations.push(operation);
}
}
Btw, there are some clippy-related warnings still fired by the crate in this state.
As we discussed, I put an alternative design here : https://github.com/arnaudgolfouse/Dijkstra_map_for_Godot/tree/alternative-operation-design Feel free to take whatever interests you !