Hi Rusty, first of all, thanks for your comments :-)
>Hi Yogi! > > "If things turn out bad". What does that mean? I guess I don't >understand this code at all. This statement refers to delaying the network synchronization to a point outside of actions.cpp and all other classes that are not supposed to deal with networking code. If you want to achieve that you might end up with having to do the synchronization at the end of the turn. I haven't examined the code in all its detail yet so i can't tell for sure. Like Bruno already said it is probably coupled to being undoable or not. Therefore if you play without fog and only move your units (like in the beginning of a game) you would have to wait until the end of the turn (things turned out bad then). One more thing i noticed when rereading my mail: I mixed up actions.cpp and ai.cpp a couple of times. What i am describing there belongs to ai.cpp. Sorry for that. Also, if you look at the current code you will find that sync_network is not within playturn.cpp yet (i moved it there when i did the refactoring). >Abstracting this into a generic "fire_sync()" event doesn't seem to >actually help. You've added a layer of indirection, so instead of it >being obvious what the code is doing, you have to grep and find who's >registered. Worse, AFAICT this sync_network() call only exists because >of networking: the programmer has to know that it wants networking to >sync here, so the abstraction is just a distraction. This is an important point. After thinking about it for a while i think it boils down to a (maybe _the_) central paradigm of object oriented design: "Classes have responsibilities. They are designed to fulfill these responsibilities best. Classes should not care about things that do not belong to their responsibilities." Lets make this more concrete: What is the responsibility of the AI code? IMO it is doing calculations of battles, identifying strategic points, placing the right units there, finding ways to maximize the damage done to the opponent. It is not its responsibility to synchronize with other network players (playturn.cpp seems to be a much more appropriate place for that). We could even argue if recording actions for replays belongs to its responsibilities but that would lead us too far away at this point. If we do not agree on this central paradigm, then it is useless to discuss ways to realize such a design. Please, don't take that as being bitter, it is not meant that way. If on the other hand we do, then we have to think about ways to get there and IMO events are one mechanism to accomplish that. If you have a different approach i would like to hear it because the current situation is far away from being optimal. One more thing i want to address here: Does the programmer of the ai code needs to know what happens when the event fires? I don't think so. He needs to know what the purpose of the event is in order to decide when it needs to be fired. So i agree with you that maybe renaming it to "fire_network_sync" would be better. But you don't actually need to know how this synchronization is done. Or how many synchronizations are done. Think of a button in a gui library. It provides a click-event that you can attach to. The library programmer has no idea what all those event handlers are doing because for the button code it is not important to know. Yes, this is a level of indirection and it is the price you have to pay for building independent components. IMO this is something desirable because the more dependencies there are, the more unexpected side effects we experience. I agree with you that it makes the code harder to understand. I remember myself struggling with hotkeys.cpp and events.cpp and it took me some time until i figured out how things were tied together. Here is, where documentation comes in, inside the code as well as separate from it (documentation? Man, i can't believe i just said that! ;-) >Confused about this example, >Rusty. I am sorry, i should have made myself clearer. I hope my explanations added all that has been left out before. _______________________________________________ Wesnoth-dev mailing list [email protected] https://mail.gna.org/listinfo/wesnoth-dev
