homoroselaps
commented
8 years ago This effects following issues:
Open homoroselaps opened 8 years ago
homoroselaps
commented
8 years ago This effects following issues:
michaelzangl
commented
8 years ago Feel free to propose design improvements.
The main cause for the split is:
Movable class takes care of everything the movable does at that moment. It can walk and play actions.MovableStrategy class tells the movable what to do next after it is done doing whatever it did.homoroselaps
commented
8 years ago My proposition is a variant of a stacked state machine. An Event represents a command, what the movable is supposed to do. An Event triggers a transition to another state, if the transition is valid. A transition to another state is done by pushing the new state on top of the stack of the state machine. The state on top is always the one controlling the movable. A state can emit events on its own for triggering state transitions. The event “Done” tells the state machine of the movable, that the current state finished its job. The state machine then pops the first state from the stack. The state, which called the last state now controls the movable. The event “Abort” tells the state machine, that the command run into problems. This gives the parent states the possibility to clean up. A carrier for example could give a transportation order back to the system if he isn't able to find a path. One after the other parent states emit the “Abort” event. This way the command stack gets rolled back. There is always one state, which is the default state of a movable. This state is only done if the movable dies. And this state would not reemit the “Abort” event if there is still a reason to live for. This insures that a movable can always react on its environment e.g. search for enemies. Every state has a lifecycle. It starts with Constructor, and ends with Destructor. Every game tick the state’s “update” method gets called and controls the movable for a small period. For example a pathing state would move the movable one step along a computed path. If a state gets reactivated because all its child states finished or aborted, the initialization is called by the state machine. Before a transition happens the finalization method of the active state is called. Each of these methods has parameters specifying the details about the transition occurred. These methods branches whether the children are finished, an error occurred. If the player gives new commands, an "Abort" Event gets triggered externally and the stack gets rolled back to the default state. Then the transition to the commanded state is triggered by the state machine. Every state has its own class. With inheritance, one can implement a simple pathing state for a carrier and derive an upgraded scouting state for soldiers, as they need to search for enemies while walking. If one needs to control a complex sequence of commands like carry. A special state is used, which accepts a new order from the system and emits events for pathing to the resource/target position, for picking it up and laying it down at the target position. The necessary information are provided with the "Carry"-Event parameters. If any of these child actions fail, the carry state gives the transportation order back to the system. Here are possible models for the different kinds of movables: Carrier/Donkey:
Ballista, Catapult, Canon
Theft
Pioneer/Geologist
Swordsman, Lance-Soldier
Bowman
michaelzangl
commented
8 years ago Seems to be nice.
I implemented something close to this in https://github.com/michaelzangl/minebot.
I always use a prioritized list of strategies. The strategy with the highest priority takes over.
That way, handling common events is easier. We could give the "die" strategy highest priority. That way, all other strategies would simply get an interrupted event and do not need to care for all cases where walking fails because e.g. a bearer cannot deliver it's goods (dead, ground was captured, destination is destroyed, ...).
homoroselaps
commented
8 years ago This sounds like its only a meta discussion whether its branched from the state pattern or from the strategy pattern. In the end they are capable of the same. Still I see some differences: In the state machine, a state can initiate a state transition. As I see it this behavior is not standard in strategy pattern. I would assume that you can only change the priorities externally and the strategy must have the information of the other priorities to trigger a specific transition. Therefore sub strategies are possible but not sound for me. With the state machine pattern there is only a single point at which all possible transitions aka the behavior is defined. This may be a Factory. This way its easy to understand and extend. The idea with the interrupted event is the same as the "Abort" event. Do you have other experiences in favor for prioritized strategies or do you see problems related to the stacked state machine?
michaelzangl
commented
8 years ago The problems I see depend a lot on the implementation.
Both have their problems. A "stacked" state machine can basically be modelled as a normal one. The design would be pretty simple but we would be left with a lot of states.
The Idea behind a state machine is to have states that are as specific as possible. We have this for workers, see IBuildingJob.
Therefore sub strategies are possible but not sound for me.
With the state machine pattern there is only a single point at which all possible transitions aka the behavior is defined.
You'll be doing a lot of case work there. All state transitions should be defined in the states themselves. Since the decision to change a state can have multiple causes, the state transition can be defined at many places.
A off-the-shelf state machine won't work for the movables because there are just too many possible states. The strategies themselves are state machines at the moment, each of them implementing it's own state transitions. I currently cannot come up with a cleaner way to do it but feel free to provide a design for it.
homoroselaps
commented
8 years ago Right now I see: State machine in Movable Strategies in Movable State machine in MovableStrategy From my point of view using a single State Machine would be a much cleaner way.
The Idea behind a state machine is to have states that are as specific as possible.
That's right. The states should encapsulate an atomic behavior. This behavior is then so general that it can be shared among other movables. My (first) model is designed with the intend, that movables share as many states as possible. This on the other hand demands for well defined interfaces, which makes it possible that the same PathingState is able to control a carrier or catapult. If you still need special behavior or optimization you can save a lot of work inheriting from PathingState for reuse of functionality. The last option would be to create a complete new on.
All state transitions should be defined in the states themselves.
I disagree, as you would introduce a lot of redundancy and lose the benefit of reusability and gonna have a hard time understanding wants going on. The last led me to start this discussion. If you add a layer of abstraction in between (aka Events), you can reuse states for multiple Movables and hock them up differently. That would be the single point transition are defined.
A off-the-shelf state machine won't work for the movables because there are just too many possible states.
Yes that's why I implemented a small StackedStateMachine. I had a look on StackStrategy and reused some of your ideas. Thx for that. As a test case I implemented the described model for the carrier. Please have a look and try the demo. My intend was to get experience with this architecture. In my opinion, such a stacked state machine would have the capability to replace the current architecture. It would achieve the same in a much leaner way.
PS: if you are into C# too, here is the C# version of it. The readme there is just a bit more verbose.
michaelzangl
commented
8 years ago Thanks for implementing that, it makes your Idea much clearer to me.
There are some things we should avoid:
IStateContructor -> Supplier<State>So your state transition would look like this:
state.deactivateState(e);
IStateTransitions trans = transitions.get(stateType);
Supplier<State> sup = e.visit(trans);
State newState = sup.get();
stateStack.push(newState);
return newState.activateState(e);First thx a lot for your feedback.
The Supplier
state == null. It adds lots of special cases. You can use a Null-Object here.
I considered it myself and it doesnt pay of. Yes on the one hand you can skip "e == null" once or twice but on the other hand you sometimes need to check whether its the NullEvent (like in StackedStateMachine.raiseEvent) In the end you create a lot of empty classes on the heap. A null(-pointer) would be more efficient. Therefore in this case I consider a null as the slightly better option.
You can use the visitor pattern here
I gone through the explanation of the visitor pattern and on the first glance our case literally sounds like a perfect fit. But the only way the visitor pattern makes sense for me would be like this: The Event as the Visitor and the State as the concrete Element. The State provides the Accept(Event) Method and the Event can do whatever it wants to with the State. But the Event has to be decoupled from the States and vise-versa to allow for reusability of the States. Or you would end up with a specialized version, of for example the PathingState, for all Movable with the transitions baked into them. Neither the event nor the state should have knowledge about the next state.
As I understand your example the Event would know depending on the active state and the Event Type itself, which state follows. But if the Donkey for example needs a different PickUp State it would need a different Event to distinguish the transitions, as the Carry State is shared among the Carriers and the Donkeys. Therefore the CarryState needs to be duplicated too, to raise the special Donkey PickUp Event. I may get it wrong. If so please provide more details about the implementation.
The point where I see a flaw is the amount of possible Events, which leads to bulky conventions. To avoid long textual definitions on what an Event should be used for, it may be better to restrict them to one new Event type per State. This way the CarryState demands for an SubClass of the PickUpState as part of a transition by raising an Event of PickUpState.EventType.
michaelzangl
commented
8 years ago A null(-pointer) would be more efficient.
Don't worry about that. Your states can (should) be singleton. You only save a few bytes by using a null pointer. The JIT will optimize all performance improvements.
The Event as the Visitor and the State as the concrete Element.
interface EventVisitor<T> {
default T visitDoneEvent(DoneEvent e) { return visitUnknownEvent(e); }
default T visitAbortEvent(AbortEvent e) { return visitUnknownEvent(e); }
T visitUnknownEvent(IEvent e);
}You can use it the other way round: Use a event.EventVisitor<T>. (we are currently using Java 7 so we should use an abstract class instead).
You would need a different state for each movable. They can share the same code. You can e.g. have a factory that creates the state that is used after carry is done.
amount of possible Events
Which events were you thinking of? I can think of done and fail, issued by the current state. We might get some external events:
Most of them can be handled by a default implementation that simply issues abort.
homoroselaps
commented
8 years ago we are currently using Java 7
Can we upgrade to Java 8, as its itself already 2 years old? Will there be breaking changes?!
our states can (should) be singleton
This is possible for the most simple ones only. The PathingState for example needs to save the target Position, the calculated path etc. Other states need even more.
Which events were you thinking of?
All commands gonna be events with this design. So we need such as: Attack, Patrol, Defend, Carry, CastSpell, GoTo, Steal, ExamineMountain ... in addition to the ones mentioned by you. Events which result in an AbortEvent can simply inherit from AbortEvent and therefore get treated as such.
You can use the visitor pattern here
I tried out your proposition to use the visitor pattern. I choose the variant event.accept(state). This way the logic stays in the states where it belongs.
Have a look.
I the end there are a lot of things which are not that lean compared to my first implementation.
First I introduced an NewEvent to have an equivalent for onActivate().
Every visitXYEvent(XYEvent e) now returns the next State which implements the transitions.
As I can only return a State or an Event, Events needs to get triggered inline with ssm.raiseEvent(new AbortEvent());. But in the case of inheritance you can not just simply call super.visitXYEvent(e) as the standard implementation of visitAbortEvent() is ssm.raiseEvent(new AbortEvent()) to rollback the state stack. So you can not use Overriding as necessary. Therefore I had to introduce the special Classes AbortState and DoneState raising an Abort- DoneEvent in the correct state transition flow.
In the end I managed to get the same behavior compared to my first implementation. But in the end quite a few tricks were necessary to use the visitor pattern in this case. (Not to be scoffed at the many hours of meditation to think through the crazy interactions and architecture) On the pro site this implementation has even less code in the classes State and StackedStateMachine and it needs less events. On the con site it needs much more states and for the states and events you are writing lots of boilerplate code. If you have an idea how to improve it feel free to start a PullRequest. Which version do you prefer and where do you see the benefits concerning our use case?!
michaelzangl
commented
8 years ago Can we upgrade to Java 8, as its itself already 2 years old? Will there be breaking changes?!
Android does not support it. You can use it for testing or prototyping but not for production code.
Events which result in an AbortEvent can simply inherit from AbortEvent and therefore get treated as such.
How do you know which events result in an AbortEvent? Isn't this for the movable to decide?
All commands gonna be events with this design
Well, there will be many, especially for all the spells of a priest ;-). To simplify this, you can create a generic UserCommandEvent. Events can have parameters like the target position or the action to take ;-)
Now about the code:
CarryState: You are using a counter here to indicate the different steps of carrying goods. This is not good style for the final design but good enough for now ;-)CarryState: Am I right that the carry state should then trigger various states that move the movable around (e.g. WalkState)?DummyState: Really helpful in my experience. Rename it to WaitState. Or make it an abstract class: TimeState and then let it extend as StandState, PlayActionState, WalkStepState, ...IllegalStateException). This helps debugging.One thing you have not done yet is to use our game timer for this. We do not tick every game tick (this is what e.g. Minecraft does and this is why people are complaining that it is so slow ;-)). The game timer can efficiently set to trigger in x game ticks. You can use this e.g. to trigger a TimerEvent. When walking, movables tell the timer to tick again as soon as they arrive at the target position. Same for playing any other animations (attack, ...). Graphics plays the right animation in between.
homoroselaps
commented
8 years ago First JavaDoc is in place, please have a look.
How do you know which events result in an AbortEvent?
You are right. Whether an event leads to abortion of the active state depends on the transitions.
All commands gonna be events with this design
Yes, there will be many. If multiple Events always initiate the same transition they can be merged with special information given in the event data. Such an group event would be CastSpellEventwith the speelId and targetPosition given in the event data. I would strongly discourage you to use a generic UserCommandEvent, because this kind of all-or-none events make the code very hard to understand in the future.
Am I right that the carry state should then trigger various states that move the movable around (e.g. WalkState)?
Yes. As of my first proposition (picture) the IdleState wait for transportation orders and then triggers CarryState(6) which controls the actions necessary to transport a resource from one spot to another. In each state, for example DropState, the Movable is controlled by the provided interface to do the action.
e.g.
public State visitTimerEvent(TimerEvent e) {
Movable m = e.context;
m.dropResource();
return new DoneState(e.context);
}An AttackState of a soldier could require a Soldier (subclass of Movable) as given context.
One thing you have not done yet is to use our game timer for this.
The idea with the TimerEventis great. But I would not use a WaitState as all states get the Movable as context and can therefore schedule the next TimerEvent themselfs.
PlayActionState
I've not yet fully understood the animation code. In my design I would not use an extra state to play animations but would leave it to the Movable to do it. If you think a state would fit the requirements it may be done by a state, too. But as we introduce more and more trivial states I fear even this architecture, but especially the one with the visitor pattern, will become a code labyrinth.
Throw an exception for senseless events
I tried it and it messes up all my tight code. I dont like the try/catch thing and because of the visitor pattern all methods may throw an exception and therefore need throws declaration all over the place. If this is good style in Java so be it.
Have a look at the worker states.
Wow there so many O.o As I see it some of them are kind of useful (DROP^^), some I'm not sure why it should be a state (PLAY_ACTION, EXECUTE...) and some I have really no clue what they are for (GROW_DONKEY, PIG_IS_ADULT ...). That's one of the issues I'm not able to refactor it myself.
The IBuildingJob looks like an Event to me. There is the information where to go and what to do. But I would split it up into two states: SearchForBuildingJob, ExecuteBuildingJob(with sub states goto, build, wait)
michaelzangl
commented
8 years ago Nice. I'll have a closer look in a few days.
In my design I would not use an extra state to play animations but would leave it to the Movable to do it.
That way, you will have an extra state machine inside the movable. If we can find a universal way for this, we should use it.
need throws declaration all over the place
You probably used a checked exception. This is something that C# does not support. Make the exception extend RuntimeException. Throwing them is good style for code that should never be reached. This helps tracking down coding errors a lot ;-).
PLAY_ACTION plays an animation for a given time.
IBuildingJob is a state class. It is the state of a Moore FSM.
The fail/success tasks define the outgoing transition edges used by the transition function. Andreas is a fan of switch statements, so the state handling logic is all there: BuildingWorkerStrategy. And the state transition: jobFinished
The state graph itself is defined in an XML file for every building.
But I would split it up into two states
I don't see any reason to do that. This is usually only done to convert a Mealy FSM to a Moore FSM. We already have a Moore FSM.
All other movables have some individual internal state machine, e.g. the digger and the soldiers.
If you want to, you can try to convert the switch in the SoldierStrategy to a class driven approach to get used to the code and the interactions that are required inside the game logic. Mind that one reason for having the enums is performance: We cannot afford to have several state objects for every movable - you can do this in the UI but not in logic which needs to handle thousands of movables. Mind that every object that is created needs to be released during GC and we have a lot of state transitions. States cannot be inlined by the JIT in your design. This is why most states need to be singleton.
andreas-eberle
commented
8 years ago Hi guys,
I just wanted to give you a short notice that I'm back and starting to read through all of this. As I'm still quite busy with other stuff, I don't know how fast I will get through all of this. Nevertheless, thanks already to @homoroselaps for contributing all your thoughts!
andreas-eberle
commented
8 years ago Hi guys,
I have some general questions. Maybe I missed it in your posts or maybe this isn't clear yet.
For the beginning, I'd like to know what you exactly consider as the states that need to be put on the stack and when they'd be put on the stack. I think it's clearer if we talk this through on an example:
Let's take a bearer that just got assigned a new carry job. This means the bearer needs to "go to" offerPosition, "take" the material from the grid (its also possible to not take it from the grid), "go to" targetPosition and "drop" the material without offering it (because it was requested by the stack it was put on).
From what I read, I think the stack would initially only contain a default state "Bearer", right?
What would you like to put on the stack when the job described above is assigned to the bearer? And when? Where would you handle a failure of the job in any of the states? (A different behavior is needed depending on the state.)
Best regards, Andreas
homoroselaps
commented
8 years ago @michaelzangl Thx for pocking me a third time into IBuildingJob. Now I finally got it: "Building" was a bit misleading to me, as it has nothing to do with the Bricklayer's job of building things. Now I see what most of the states are for. IWorkerJob would've been a better choice from my point of view. I read that the Java GC is optimized for creating many small objects. But even in C# I have no experience in situations where the GC is the bottle neck. You stressed out that its important that the states are singleton. In Digger/Soldier strategy every state reuses the variables from the last activation, to spare the GC. But the solution of dumping the logic of all states into a single class is barely the best solution to handle this, as it's hard to understand and extend. We may call the static methods of the (singleton) states always with a context data parameter. (This would enable great multi threading opportunities or even functional programming). The context objects of all states on the stack are saved within the state machine. A Dictionary with the State-Class as key may be the way to go. (This prohibits recursive state transitions, which I consider bad style anyway). Any other ways to cope with this?
@andreas-eberle
From what I read, I think the stack would initially only contain a default state "Bearer", right?
YES. In my design I call it IdleState.
What would you like to put on the stack when the job described above is assigned to the bearer? And when?
When the job is assigned to a bearer. The Bearer Class raises a BearEvent (called CarryEvent in my design) with the complete job description on the internal state machine. This triggers a state transition from the BearerIdleState to the BearerBearState. The state transition consists of pushing the new state on the stack.
Where would you handle a failure of the job in any of the states?
If a failure occurred and a state needs to abort, it raises the builtin AbortEvent. By this means the state machine always pops the topmost state of the stack and therefore activates the parent state with the AbortEvent. This state has then the opportunity to handle the failure or raise an AbortEvent itself and so on... The root state (e.g. BearerIdleState) should always stop propagating AbortEvents. The builtin DoneEvent pops the topmost state of the stack as well, but communicates a success.
andreas-eberle
commented
8 years ago I have no experience in situations where the GC is the bottle neck
On PCs there is usually enough power so you don't really have to care. However on Android phones, this can become a real issue besides memory consumption (which is currently our limiting criteria).
We may call the static methods of the (singleton) states always with a context data parameter. (This would enable great multi threading opportunities or even functional programming).
I don't think this would enable multi threading. This is mainly because the "context" of the state/movable includes the whole grid. Because the movables (potentially) interact with each other in most states, you cannot simply ensure a deterministic behavior (which is required by our multiplayer and debugging concept) when executing movables out of order.
Furthermore, I don't think a context object is a good solution. This would either require different context objects for every movable type or require us to create a huge "fits all movables types and states" context object. That, however, would not be easy to understand and also huge in memory.
This triggers a state transition from the BearerIdleState to the BearerBearState.
What would the BearerBearState do? Judging by the name, the state would itself contain a state machine coordinating the steps of the carrying. Is that right?
If a failure occurred and a state needs to abort, it raises the builtin AbortEvent. By this means the state machine always pops the topmost state of the stack and therefore activates the parent state with the AbortEvent.
In general, this seems ok. However, you need to consider that the parent state cannot do the cleanup. Only the aborting state knows the exact state which is needed to correctly abort the current action. E.g. for a carry job of a bearer the following cleanups need to be considered in the intermediate states
homoroselaps
commented
8 years ago I don't think a context object is a good solution
What are the alternatives? In the current version a context object per movable is used (the sum of all strategy fields). In my opinion a context object is obligatory in our case, as the PathingState for example needs to save a calculated path or the target position to work correctly.
What would the BearerBearState do?
Yes, its would contain a small state machine or pipeline. see line 95 of my prototype
Only the aborting state knows the exact state which is needed to correctly abort the current action
This made me think for a while. My assumption is that every task clears up all resources (this may be an accepted transportation job) on deactivation by DoneEvent/AbortEvent. In the end we don't want/can rollback the entire action like in a database, but just go into the nearest possible state where no resources kept locked. Problems occur, when you have multiple steps (like carrying a resource) which together form an atomic operation. For example the transportation of a resource: GoTo1, PickUp, GoTo2, Drop. If an Error occurs in Goto2 the PickUp State had already finished and was poped of the stack. So it can't give back the picked up resource to the system. We could use a solution which ensures that all sub states of the pipeline remain on stack until all are done. What would be your solution? In the movable we have many states not captured by this state machine: Whether or not it holds an item, whether or not its in a tower, whether or not its visible etc. The movable could be in multiple of these states at ones. We have to consider the states of this "state machine", too. This should be the job of the RootState (eg. SoldierIdleState, SoldierIdleTowerState) which always set meaningful default values. Do you have any other ideas how to gracefully handle abortion of an action?
Why would you treat the transport as a success if something goes wrong? The definition of the AbortEvent is the opposite. Otherwise the 4 cases you described are not so different. To sum them up:
andreas-eberle
commented
8 years ago What are the alternatives? In the current version a context object per movable is used
In the current version, the context object is movable type specific (the strategy holds the parts of the context that are type specific). In your proposed idea, I would let the states have attributes (in contrast to Michael). Therefore the states some of the states currently on the stack would be the context. I don't think the creation of the state objects will be that bad. The numbers aren't that large.
Yes, it would contain a small state machine or pipeline
OK, so to sum it up, the change is to extract "common" states out of the strategies to reuse them. Therefore the strategies behavior would be shifted into the
Correct? (I just want to make really sure we're on the same page with your knowledge about your idea and my knowledge about the current system, so we can develop something that's really an improvement. ;) )
but just go into the nearest possible state where no resources kept locked.
Correct. That is what is currently done by the handleJobFailed() methods in the strategies.
What would be your solution?
I see two ways depending on what the carry job is actually doing:
So option 2 seems the only one to go for, or do you think different?
In the movable we have many states not captured by this state machine
At the moment it is the idea of the movable class not to care about the detailed states of the strategy. The movable class only executes common movable operations. So if it gets the task to go somewhere, it can do this without caring about the movable strategy and in which state that is. The strategy will only be called again, when the path is finished and it can continue with its own state machine. So like I wrote before, in your design, the movable strategy will be split into the IdleState handling the "get a job" stuff and into the "execute a specific job" states.
Why would you treat the transport as a success if something goes wrong?
For example: What happens if the bearer dies/fails (just as an example; reason doesn't matter) at the position it should bring the material? It drops the material and therefore the request is fulfilled. So isn't this a success from the job's perspective? If you do not handle that job as a success, but as a failure, the system will see there is a new offer which is very close to that request that was just put back (distance 0) and assign that job to a new bearer. That bearer will walk there and pick up the material, only to drop it off again.
Now you could say: Why you don't handle that special case in the job management? You could see the distance is 0. Well, the job management has to handle all offers and all requests. This check would need to run for every job assignment to a bearer; or better for every incomming offer/request. It is much more efficient to handle that special case where it happens, instead of making another large system handle something it doesn't care about.
Otherwise the 4 cases you described are not so different.
It was only an example, I'm sure there are / will be more complicated state depending failure handlings. However, with option 2, we can handle them like it is done at the moment (which should be fine).
One more thing, I want to point out:
At the moment, I don't think this refactoring (which could really make the code clearer) will help making the pushing problem and handling easier. This is mainly because you don't want to abort a movable's current job when it is pushed. Instead, you want to find a way to react on being pushed while continuing the movable's job. However, the reaction to beeing pushed doesn't depend on the strategies state but only on the movables state (going somewhere, dropping, taking, playing animation...). In the current system, the pushing handling is implemented in the Movable and is therefore independent from the strategy's state.
We can probably transfer this system to the atomic states on top of the stack (GoTo, Drop, Take...) and let them handle the push (again splitting up the switch-case). However, as some states share the same handling, we might end up with the problem to share code. (Of course we can do superclasses and so on...) I just want to make you aware of this.
homoroselaps
commented
8 years ago Thx for all your thoughts!
What are the alternatives? In the current version a movable type specific context object is used
If you say all the objects wouldn't be a problem - that's great. In my "final" (most recent iteration) version of my prototype I use static states for better performance. I updated my master branch. In the end I need the functionality of a complete double dispatch. In classic implementation this would result in a lot of boilerplate code (with all known problems). I found a solution with reflection and complex state transition costs about 600ns. That's half as fast as my classic double dispatch solution and by my estimation acceptable for our use case. (Please check out the "bench" option of my prototype).
Yes, it would contain a small state machine or pipeline
These "state machine"-states are a extreme case and may be generalized with a SequenceState superclass. I considered your solution, too but I didn't find an elegant implementation for the "put the whole sequence an the stack". In the end this doesn't fix the clean up problem but makes it more complex as you said.
You said "split into the IdleState handling the "get a job" stuff". I assumed that the job management assigns a job to a bearer - so he doesnt need to "get a job". But of cause you're right for the soldiers as they look out for enemies and then get themselves a job.
If you say "the behavior of the strategy moves to the IdleState" it doesnt sound 100% right to me. The behavior gets split up as much as possible into atomic (reusable) sub actions which become separate states. This way it would be possible to easily create a SoldierBearer just by reusing states from Soldier and Bearer, plus one or two very specific ones. Bugs may then be only specific to the movable type or the current action/state. The first results from a faulty configured state machine. The second results from bugs in the state's implementation.
With my prototype we have great power and therefore great responsibility in creating a meaningful state- and event hierarchy. In my current design of the state machine I already introduced a SuccessEventand FailureEvent subclass of the too generic DoneEvent. This way a state completes either with a Success/FailureEventor with a more specific subclass of these. This gives more information to parent state.
The AbortEvent is only raised from outside the state machine to abort the whole action stack. For example if a soldier gets a new command, he needs to clean the stack beforehand with an AbortEvent. He returns to the root of the state graph and then enters an other subtree of the graph based on the Event/Command.
Every root state represents a distinct state of the movable. For example it is essential for a soldier to differentiate between IsInTowerand IsOutsideTower. For example a soldier cannot patrol if he is in the Tower. I am totally aware that this solution doesnt scale very well as you need the power set of all possible states. (Tower_Selected, NotTower_Selected, Tower_NotSelected, NotTower_NotSelected) :fearful:
But I already have some ideas how to solve it leaner if necessary.
If a movable changes the internal state, for example while entering a tower, the RootState needs to change.
For this case I introduced a special transition type which clears the whole stack beforehand and requires a subclass of RootStateas target. (a different solution would be to hot swap the root state)
Why would you treat the transport as a success if something goes wrong?
I dont see the need to threat this as a special case as long as the system remains in a valid state and as long as they dont occur frequently. If the bearer dies and the resource is at the right place by chance - I would just say "shit happens" xD. I understand that this was just the most plausible case for you. But as far as I see this gonna be one of the more complex ones. I am confident that we can handle most cases properly this way.
However, the reaction to beeing pushed doesn't depend on the strategies state but only on the movables state
Yes, I like michael's idea very much of merging the movable state machine into this model. We will get states for Pathing, Playing_Action and Pushing. This way my initial pain point would be solved, as the pathing logic becomes movable type specific. Whether a movable IsPushable can be designed as a transition to the PushState in the graph. This way you decide by design which action can be disrupted by pushing. For example if a movable is Fighting you can deny it, if he is pathing and got stuck (refer to my motivation picture from the beginning) it may be allowed.
The possibility to build a state hierarchy is an essential part of this OOP approach. This way you can implement a default PathingState and adapt it for other movables. The migration to the new architecture gonna be great bunch of work.
michaelzangl
commented
8 years ago Some performance hints:
IdentityHashMap to speed things up more ;-)containsKey and then get. Simply do a get and check if the result is null.homoroselaps
commented
8 years ago Thx michael for the hints. On average a state transition is now about 100ns faster. In my very simple benchmark the IdentityHashMap performed a bit worse (-50ns ± 150ns) than a normal HashMap?!
michaelzangl
commented
8 years ago I never tested it for classes. It is normally faster than the normal HashMap since it uses identity. I can have a more detailed look at it but I think that the JIT just optimizes the class to basically be an IdentityHashMap - since it realizes that classes are only compared by identity.
To test it more reliably, run the following code before your benchmakr, to mark the paths for the JIT so that it does not inline hashCode and equals in get by giving it two additional execution paths:
HashMap<String, String> a = new HashMap();
HashMap<String, Integer> b = new HashMap();
for (int i = 0; i < 100000; i++) { a.put('x', ''); a.get('x'); b.put('x', new Integer(1)); b.get('x'); }Same for the identity hash map. (benchmarking in Java is really difficult - the JIT destroys your best benchmark code :D)
homoroselaps
commented
7 years ago Hej, right now I am working again on this issue and would like to start a little discussion about my plans:
The current Movable implementation grew over the last couple of years and became a "blob". It consists of many fields and functions that only a handful of types of movable actually use. I want to introduce the Entity Component Model as a way to encapsulate Data and related Logic. This way it should be possible to improve parts of the Logic ( eg. Pathfinding ) and implement the missing functionality ( ship, magician etc.) without running into the current problems of side effects.
I want to use Behavior Trees as a substitution for the complex state machines. For simple behavior they are as lean as FSMs but scale much better and stay readable for complex behavior such as needed for the soldiers. My final goal is use an Event Driven Behavior Tree as they are more efficient and cleaner in implementation. But until lambdas are not available I will go with the simpler variant.
This is new territory for me and I am open for any suggestion or critic. In particular in terms of the Entity Compontent Model I have not yet found an implementation, which fits well into the existing environment. As this refactoring is an enormous chunk of work, I would be very happy for any support. I plan to finish this until the end spring.
PS: I'm working on this branch PPS: My prototype for the Behavior Tree: https://github.com/homoroselaps/BehaviorTree
I was not able to debug movable behavior as its much to complicated. I was trying to refactor it myself but there are to many side effects and the responsibility/feature list of movable is not clear. For example function timerEvent in Movable.java has 3 switch statements and many fallthroughs. In addition MovableStrategy adds states on top of movable's states which doesn't end well. Picture for motivation: