- Cleaner code
- No break; necessary
- Each event type is his own function call
- Same performance
- Possible to change function event at runtime
nani on Mar 16 2019, 7:29 PM.Authored by
Everything should behave as before.
There are a very large number of changes, so older changes are hidden. Show Older Changes
Now would be a matter of improving all the helper functions that the FSM calls.
For now this patch moves:
Transforms it to:
The fsm.js has been modified to be able to have a default function and "_" in state name
What uses the underscore?
Was it a lucky try? Because FeXoR is primarily a mapauthor, but I think he worked on the wallplacement code in this file, so perhaps you found him using the revision history. Might suceed with a ping @, but we have people who are more dedicated to the JS GUI, Imarok and bb would be a good bet, but one of them has broken bones and the other one is studying. Vladislav is another option, but he is also busy with various stuff. I guess I know another option to have this patch committed that involves me not committing it, but that would be new.
None but given that the state can only be written with uppercase it might serve in the future as a word separator given that it doesn't conflict with the naming of the methods (but anyway I can just revert this change for now)
The default allows to ignore all the events that one might not want to process but still happen (e.g. if in a state "PLACING" you only want to process "onmousemotion" event and ignore all the other types of events) otherwise the FSM will warn you every time you try to process an event in that state that is not defined. Mostly the default method should be an empty function in this FSM case (unless we are talking about those two "default" that I had to define at "PRESELECTED" and "SELECTING.BOX" states given that I could not find a way to refactor them to non-defaults methods.
So one of the problems is that all input events such as keydown, mousedown can be triggered, the FSM model expects all these to be defined, but we don't want to specify tons of empty functions? Would it actually be that many empty functions? Some empty lines here and there wouldn't hurt so much and it could possibly yield a more complete picture. Empty call handlers could also be considered as an incentive to think about filling them.
The events have entirely different arguments (keydown the button, mousemove the coordinates), so the arguments of the default function is unable to consume the provided argument.
Isn't InputEvents.prototype.events.default unneeded either way?
Why are unit actions (patrol, gather, ...) mixed with input events (keydown, mousemove, ...) for some objects?
(I meant whether you had found him by looking through the authors of this file)
You're welcome. As mentioned in a previous post, I post my comments to help educate about good practices, but I'm currently not working on code until I fixed some problems with working on code that I had over the years.
Preliminary note: If you can, move the non-refactoring changes outside of this diff. It's big enough. Git is convenient for this.
I am not sure this is the right way to go about refactoring input.js. The switch-as-FSM isn't the big issue here, the issue is that everything is a global and it's a huge mess.
Since all actions appear revert back to "Normal" (i.e. none switch to another state / at most they switch to one of their own sub-state), I don't think we need an FSM.