Advise Please 2.0

[a10e29]

Sorry for partially quoting your words.

No worries. I'm probably guilty too.

Am going to start working on this myself, haven't ever implemented one of these before - will give it a Larrop just to see if I can figure it out :+)

I've failed at it, have not given up though. The first try generated some 219 errors and had polymorphism. The current version blows up if class Event has #include "Hex.h" or anything else that can form a sender/receiver relationship (wanted this to be the filter for now). It's only two classes at the moment and I'm 99.999999999% sure it won't work as is. So not worth sending an update quite yet. Hope you have better luck than I did.

But it would be good if we could avoid using new and delete, try to use the STL as much as possible, or smart pointers where necessary.

Absolutely agreed.

Another issue is the use of exceptions, while not a bad thing, if we are going to have them we would use them everywhere.

It's worth looking into, it's too big of a (lack of) system to completely ignore.

While we're talking about hard to retrofit things, in a PM this was suggested:

I would use Unit Tests from the beginning and also a logging system. They make testing and debugging much easier

Honestly I've been too busy to research them in any depth. You guys know much about these? The part that sold me on it was

They make testing and debugging much easier

Regarding POD - go for it.

I'm going to localize this until I've got it perfected.

Going to start posting a link in the OP to the bottom so don't have to torture our mice/fingers so much :)

[TheIdeasMan]

Hi,

Sorry for my late reply, I had more of other stuff to do on the weekend & Mon Tues that what I had originally envisaged, so I haven't really started yet.

The current version blows up if class Event has #include "Hex.h" or anything else that can form a sender/receiver relationship (wanted this to be the filter for now).

Not sure I understand any of that :+( I had a look at your Hex class, it seems to be location oriented - I am mystified as to why you would have that mixed up with events, subscriber, publisher and filters? The Event class is supposed to be an abstract base class for the other events to derive from. Have a look at the Java code.

Location type stuff with maps & terrain etc, is another thing further down the track. When we do have that, things related to location can be sent as a message in an event. At the moment my goal is to just to get simple combat working.

It's only two classes at the moment and I'm 99.999999999% sure it won't work as is.

Looking at the UML diagram for the Event Notification Pattern, I count at least 7 classes needed to implement it, but you only have 2?

With the exceptions, I mentioned that because the Java code shows throwing an exception. So that is a application wide design decision. Exceptions are a good thing IMO, we should have them.

I would use Unit Tests from the beginning and also a logging system.

You guys know much about these?

Sounds good. I haven't any experience with Unit testing, but I gather it about writing code to test your code. Hopefully not too hard to implement a logging system, either. Not that I have done a proper one of those either.

Btw, have you had any PM's from anyone other than dhayden ? Just that if there are multiple people it would good if we could all collaborate.

Edit:

Going to start posting a link in the OP to the bottom so don't have to torture our mice/fingers so much :)

Not sure why you would do that, in the forum where your topic is listed, there is a link to the latest post, and links to each page within the topic.

[a10e29]

Hi,

I had a look at your Hex class, it seems to be location oriented - I am mystified as to why you would have that mixed up with events, subscriber, publisher and filters

Event is going to have to have some kind of information attached to it, one type or another. Consider movement, going from hexA to hexB, in your opinion what information would be relevant in that particular case?

Regarding Publisher and Subscriber, Faction and Piece are both candidates for being publishers and subscribers, on different levels. i.e. Piece subscribes to the EventService faction publishes to while piece publishes to the EventService soldier subscribes too. I am over thinking this?

Sounds good.

Great! It seems pretty daunting but well worth the experience.

Btw, have you had any PM's from anyone other than dhayden ?

Thomas1965, who we have to thank for the unit testing and logging suggestion. He provided some very informative links on unit test as well:

Here are some tutorials about GoogleTest. Have a look if you have some time. In the long run unit tests will make your life easier.

https://www.youtube.com/watch?v=TS2CTf11k1U&list=PL5jc9xFGsL8GyES7nh-1yqljjdTvIFSsh

http://www.codeproject.com/Articles/696481/Quick-start-unit-test-How-to-start-working-with-th

About CPPUNIT

http://www.codeproject.com/Articles/5660/Unit-testing-with-CPPUnit

Psychology of Testing

http://www.codeproject.com/Articles/17649/The-Psychology-of-Testing

On the data front have two PODs (one shown) here they are:

#ifndef DATAFORSOLDIER_H
#define DATAFORSOLDIER_H

	struct rankParams {
		int quality;
		int baseAttack;
		int baseDefense;
		int hits;
		int numberOfAttacks;
		int initative;
	};

	static rankParams rankMap[] =
	{
		{ 0, 2, 2, 1, 1, 4 },
		{ 1, 6, 4, 6, 1, 8 },
		{ 2, 8, 8, 8, 1, 10 },
		{ 3, 10, 10, 10, 2, 12 },
		{ 4, 16, 16, 6, 2, 15 },
		{ 5, 18, 18, 8, 3, 20 },

	};

//ect.

#endif 

More or else copied your example dhayden. I've begin to suspect that there is a better spot for these where I can take advantage of my enums without creating circular dependency. On the bright side Soldier has no switch statements. I do get an error when I try to remove the older ctors, what stands out about it is: Soldier(Soldier &) is this a rule of five issue?

Not sure why you would do that, in the forum where your topic is listed, there is a link to the latest post, and links to each page within the topic.

I can officially say I learned something new today

[TheIdeasMan]

Hi,

I still haven't finished the Event Notifier, have been also working on a Business Plan and associated stuff. Will get some done tonight though.

I am over thinking this?

Probably more of getting ahead of yourself, if you don't mind me saying so :+) It's like you want to go car racing: but the chassis, engine and transmission are all separate parts, despite that, you are trying to get the air-conditioning, telemetry, and brake bias working.

I hope you view all of the following as constructive criticism:

Maybe time to reiterate my philosophy on this. Einstein once said: "Things should be simple, simple as possible, but no simpler " . Often I find that people trip over on the last part, but here I think we need to start at the simple end. So start with something very simple, get that working, then move on to more complexity after that.

The reason for the simplicity is twofold: You had a 25KLOC with lots of complexity, and which had unworkable design and code issues; It's easier to start simple and have it working and easy to extend, rather than start complex and finish up with it not working. Even if we had a complete design with 100 classes say, I would still get small parts of it working first.

With your code, I have said a few times already that the new code won't look anything like the original code, and I don't see it as being worthwhile to change the original code. This is probably hard to accept: you probably spent weeks doing it, now I am saying to tear it up and put it in the fire; or keep it for comparison purposes, old to new, to see just how much difference there is. So I am trying to steer away from criticising or changing the existing code, apart from ideas of doing things in a better way for the new code. Just on that: I reiterate the names you have IMO are terrible. Piece implies an individual thing, I would have called it Squad. What is Hex? You mentioned going from HexA to HexB, do you have hexagonal tiles on a map? To me that immediately sounds suspect / over complicated. Anyway those issues are in the future. Faction (whatever that is) is another thing to deal with later.

My idea at this stage is to implement the Event Notifier with everything it needs, and use it to do some very simple Combat: One Soldier fires at 1 Enemy until it dies. Then it should be easy to have 2 way combat, perhaps implement Armour, then combat with multiple targets, then more detailed Command events.

So to that end, in my mind the publishing of an Event, would have Sender, Receiver and the Event itself as arguments. The event itself can have information inside it, like the amount of damage the attack represents, or much later in the project things like Location.

I hope you realise that the Event class is an abstract base class, with the polymorphism one sends a pointer to a derived class to the function or container.

My idea was to have the Actors to have a Combat object, which would be both a Publisher and Subscriber to Combat type events. It owns the Weapon, Armour and Health objects for the Actor. It would also Register itself as being a participant in the Battle, and publish an event when it dies. I am also going to have a Command object, which commands the Soldiers (or groups of them). The Command object subscribes to Registration events, so it can assign an Enemy or Enemies to a Soldier. It can publish events to initiate the Battle. The Command can also publish movement events.

So, to get you thinking about this: What are the classes you are going to have for the Event Notifier, The events themselves and anything else needed to get it working? Hint look at the UML diagrams and the Java code in the documentation.

Thomas1965, who we have to thank for the unit testing and logging suggestion.

Ok, awesome :+). It is good to know what everyone is suggesting, just so we don't become at cross purposes to each other :+) AFAIK the unit testing, logging and exceptions shouldn't be too hard.

On the data front have two PODs (one shown) here they are:

While trying not to go directly against what dhayden is saying (it might be your interpretation of what he said), I see that information as statistics. Even though it is to do with the Soldier, IMO it should not be directly in the Soldier class. Like everything else, it could exist in a separate Statistics class/object which a Soldier can own a pointer to. The Rank could still just be a string or enum, but the value of that is determined by the statistics. I am not sure you fully understand that concept yet, here is a further detailed example:

Say we have a Person class. It needs to store info about names, date of birth, address, driver's license, passport, swimming club membership etcetera. Instead of those things being strings, they would all have their own class and the Person stores pointers to those objects. Abilities to drive or swim would also be in separate objects which the Person would own pointers to.

It might be a surprise, but there are often ways of breaking things down to smaller classes. For example, Scott Meyers (Author of Effective C++ series) not only had a Date class, but he had a class for each month ! He reasoning was to make the class easy to use properly and hard to use improperly. The ambiguity with dates is the varying day/month , month/day formats around the world.

The thing is, one has to think in object oriented terms. This sounds obvious for OOP, but it is a completely different paradigm and mindset to procedural programming (PP) like one would do in the C language say. PP tends to be data centric: data in, some process, data out. So this is very different to OOP, where objects model the real world, or relationships between abstract concepts.

[a10e29]

I hope you realise that the Event class is an abstract base class, with the polymorphism one sends a pointer to a derived class to the function or container.

I do. I also realize that the handles will have to have specific types:

Something::handle(EventType18*);
Something::handle(EventType99*);

ect.

So, to get you thinking about this: What are the classes you are going to have for the Event Notifier, The events themselves and anything else needed to get it working?

EventService
1. a queue of Event*
2. a queue of sender*/receiver* pairs
3. vector of subscribers*

Event and EventDerived
I'm still deliberating what will constitute as valid data for these.
Edit: combat for instance would it be better to have a single event that contains all the data for successful hits (in vector or something) or for each hit a new event. that sort of thing

Filter
I'm a bit confused on this. If we establish a sender/receiver relation would it be self filtering?

Publisher already have this, all it needs is a way to generate the appropriate event and a shared pointer to Eventservice. Eventually a way to not generate certain events on it's self.

Subscriber already have these, needs a handle for each event type.

It would also Register itself as being a participant in the Battle, and publish an event when it dies.
Why not just unsubscribe? Will the publisher need to listen for events for when to erase the dead thing?

[a10e29]

Event Notifier:
https://www.dropbox.com/sh/dm7jw49q7i13qpn/AABAVNTcFbJ82hR0VqMH4Moxa?dl=0
It works for random events. haven't mastered publishing quite yet

[TheIdeasMan]

Hi,

I have been off the air for a bit, been in hospital had appendix out :+)

Have looked at the code in you PM. It seems you have implemented part of the Event Service, but not much of the other things I have been talking about, but the worst part is that you have mashed it in with your existing code. There are lots of essential (IMO) classes missing. What's going on here? Is it that you don't understand all the long replies I made; or for some reason are choosing to ignore what I have said; or is that you are still desperately hanging on to your original code? I have spent all this time explaining some good design ideas, and not much of it seems to have sunk in. Sorry to be so blunt, but we don't seem to have made much progress.

So, to get you thinking about this: What are the classes you are going to have for the Event Notifier, The events themselves and anything else needed to get it working?
EventService
1. a queue of Event*
2. a queue of sender*/receiver* pairs
3. vector of subscribers*

1. EventService has a container of Subscriptions, whether it needs to be a queue is another story - the whole thing will be iterated anyway. I was going to make it asynchronous by having it's own thread.

2 & 3. NO!!!, dammit :+) There is no need to store tables of data like that, just call the functions in the class. We want object oriented, not data oriented.

Edit: combat for instance would it be better to have a single event that contains all the data for successful hits (in vector or something) or for each hit a new event. that sort of thing

The latter, every time someone fires a shot, it's an event. Again, object oriented, not data oriented.

In terms of storing statistics data, individual objects can do that - The Event Service is certainly no place for that.

Filter
I'm a bit confused on this. If we establish a sender/receiver relation would it be self filtering?

Imagine we have multiple Soldiers firing at multiple Enemy. A Soldier (rather it's Combat object) would have a dynamic list of targets. The way it receives events is to first filter out only those events which are directed at him, otherwise he processes hundreds of events that have nothing to do with him.

Publisher already have this, all it needs is a way to generate the appropriate event and a shared pointer to Eventservice. Eventually a way to not generate certain events on it's self.

Subscriber already have these, needs a handle for each event type.

You have a Soldier class, no Actor class, no enemy class, and I don't see a Combat class which the Actor should own.

It would also Register itself as being a participant in the Battle, and publish an event when it dies.
Why not just unsubscribe? Will the publisher need to listen for events for when to erase the dead thing?

If the dead Enemy unsubscribes , then the Soldier has no way of knowing until an exception is thrown because a combat event is sent to something that doesn't exist. We want to avoid exceptions for that sort of thing. Better to receive an event that the enemy is dead, so Soldier can remove it from it's target list. The publisher doesn't erase the dead thing, things should erase themselves when they die.

[a10e29]

Hi,

Proposed new class structure, it's really rough. Fairly sure I've got some of the components screwed up. The plan is not to do any coding until I've got a visual aid as to what I'm doing. the ones with an X by or in their arrow come later (or possibly not at all)

https://www.dropbox.com/s/e41hz5z3p3egtdu/Proposed%20UML.bmp?dl=0

Things to note on that:
I intend to recycle Hexagon_Grid, Hexagon, and SDLToolBox (SDLT), the former two do work albeit there is room for improvement there, the latter is a mess but functional.
edit:
Regarding the former two are there any immediate improves you could suggest here? I have a few ideas of my own but I'd like to hear your input
refactored
Regarding SDLToolBox the plan is to make it like a tool box (distinct tools)

but the worst part is that you have mashed it in with your existing code.

That was intentional :), the existing code has become a means to gain insight into how event notifier should work (because it could be visually confirmed it was working) and figure out good combinations of statistical structures (IMO they should be as small and specific as possible, it's easy to forgot something in the bigger ones) Sorry for the partial quote.

Some coding questions for future reference:

class Foo
{
virtual void doSomething() = 0;
}
class Bar : Foo
{
virtual void doSomething() =0;
}
class Baz : Bar
{
void doSomething();
}
std::vector<std::unique_ptr<Foo>> vect;
vect.push_back(std::make_unique<Baz>);

Would that work?

Other question, how would one restrict class norf from being able to create a baz object but not a qux object and visa-versa?

[TheIdeasMan]

Hi,

I don't see your picture. It probably doesn't matter though - Again I seriously think we should start out simple. So I reiterate my Idea from much earlier: Have a Player shoot at an Enemy until dies. Could do it without the Event Notifier, just make sure there is a reaction from the Enemy using std::cout. Dead Simple. Once we get that working, we can add more features and deal with their complexities one by one.

I think what can happen is that someone has learnt a procedural language like C, or something else, then try their hand at C++. They can go on and learn all kinds of things about the language itself, STL, boost, whatever, but despite all that, they don't have a grasp of what it really means to do OO design properly. It's an entirely different way of thinking. Not that I am really good at it or anything, but I do have some ideas.

We (the other respondents) recognised this right at the start, and that is why we suggested starting again, and I suggested starting simple then work upwards.

So I am not going to comment on any of the other stuff until we have the basics right. In terms of the car analogy, we want to put together the chassis, engine, transmission and body; painting and window tinting come much later.

Sorry for the partial quote.

I only mentioned that in case there might have been some confusion or misinterpretation coming from it, no need to continue to apologise :+)

With the code, what happens when you try it? Shouldn't need pure virtual function in Bar, If one doesn't override the parent virtual function, then Bar will be abstract too - it's incomplete. Use the override keyword, when you do override - it's helpful.

Does the push_back work for the unique pointer? It normally tries to copy, which is an error. I would expect to use std::move to get it in there.

Other question, how would one restrict class norf from being able to create a baz object but not a qux object and visa-versa?

Apart from not including their header files, I don't know what you mean. Need to see real life context.

[a10e29]

Hi,

With the code, what happens when you try it?

Honestly haven't tried it yet. Neat! Is that possible because Bar : Foo? (It makes a Foo object in the Bar ctor)

If one doesn't override the parent virtual function, then Bar will be abstract too - it's incomplete.

That's some good information.

Does the push_back work for the unique pointer?

Yes I do it all the time no problems. most of the time no problems:)
example from the old code with out any trouble

formations.push_back(std::make_unique<Combatformation>(cities.at(0)->getLocation(), grid, cities.at(0)->tool, roleParam, rankParam));

It normally tries to copy, which is an error

From what I've observed:

vect.push_back(std::make_unique<Foo>()); // no copy, depends
std::unique_ptr<Foo> f = std::make_unique<Foo>();
vect.push_back(f); // error! tries to copy 

I would expect to use std::move to get it in there.

Always wanted to try something like this:

std::unique_ptr<Foo> f = std::make_unique<Foo>();
vect.push_back(f.release());

http://www.cplusplus.com/reference/memory/unique_ptr/release/

Releases ownership of its stored pointer, by returning its value and replacing it with a null pointer.

It seems like it would work, but that might just be me misunderstanding.
Yep I misunderstood, it returns a * not aunique_ptr

Apart from not including their header files, I don't know what you mean. Need to see real life context.

Okay when I get to that point I'll ask it again with better details.

[a10e29]

Hi,

A Soldier (rather it's Combat object) would have a dynamic list of targets. The way it receives events is to first filter out only those events which are directed at him, otherwise he processes hundreds of events that have nothing to do with him.

Quick questions here:
The combat object is going to do the publishing with the destination being an Actor?
The dynamic list of targets will be type Actor?

If the dead Enemy unsubscribes , then the Soldier has no way of knowing until an exception is thrown because a combat event is sent to something that doesn't exist

Is there A good way to make an event of a higher priority than other events?

Just want to make sure I have this straight before I jump in.

[TheIdeasMan]

Just want to make sure I have this straight before I jump in.

Can I convince you to put your big toe in first ? :+) It is clear from your questions that you don't yet understand the design decisions made so far.

TheIdeasMan wrote:

So I reiterate my Idea from much earlier: Have a Player shoot at an Enemy until dies. Could do it without the Event Notifier, just make sure there is a reaction from the Enemy using std::cout. Dead Simple.

Once you have that, then could make it so there is some delay between the shots, and in changing magazine (quiver, whatever) incorporate attributes in the Weapon class to achieve this. Still no event notifier.

Some questions of my own, might help you answer yours:

Why do we have an Actor class?
Why do we have Player and Enemy classes?
Why do we have a Combat class?
Why is Combat a member of Actor?
Why do we have a Weapon class?

Some of these are to do with what arguments a function takes, or a container holds, others a major feature of OO.

Is there A good way to make an event of a higher priority than other events?

Why? Do you remember the code in the documentation? Not sure whether you need a higher priority. Get the simple things right first.

[a10e29]

Hello,

Can I convince you to put your big toe in first ?

Yes. Glad you did. I modified the old code to accept the new events. I left the new code in a state in which it could go either way. That being said I no longer like the idea for event notifier on the actor scale of things for multiple reasons. I'll explain later.

Why do we have an Actor class?

Purely a design choice. It could be an int in the Gamepiece class that represents how far it is from being destroyed.
As for what it should do: it needs to be an abstract class that is responsible for doing everything that all derived classes have in common and providing an interface for the derived classes that do something similar but different things. IMO it should be a meta class with only data members that are objects.

Why do we have Player and Enemy classes?

The responsibility of the derived class is to define the interface provided by the base class and add additional functionality if necessary. Note that this additional functionality is dependent on 1 of 2 things otherwise it is inaccessible:
1. it's new functionality is part of it's interface with it's base class
2. the derived class is downcast from it's base class in which they can be accessed directly.

Why do we have a Combat class?

Combat class is responsible for the interaction of Actors in one specific context.
In other words it is what they do to each other.

Why is Combat a member of Actor?

Combat is one of the responsibilities of Actor, because it is a fairly complex responsibility is why it get it's own class.

Why do we have a Weapon class?

Weapon is responsible for containing the min/max damage, how fast it is, how long it takes to get ready, ect. It is NOT a member of Actor because such data has literally no use to help Actor achieve it's responsibilities. It is a member of Combat because it does help Combat achieve it's responsibilities.

Once you have that, then could make it so there is some delay between the shots, and in changing magazine (quiver, whatever) incorporate attributes in the Weapon class to achieve this.

I've played with the idea of an RTS/TBS hybrid where you get 20 seconds to do whatever but have each action take 30 seconds. It is an interesting idea but not for this project.

Regarding Event Notifier:
How I wanted to use it was instantaneously generate 99 % of the events. When there was a new death event, in a separate container, remove all events where it was either the sender or receiver. Like I said earlier, I modified the old code to accept the events in the new code and found it very lopsided regardless of how old code treated quality.
Things I observed:
1. events directed towards it could have been wasted (a sender basically did nothing if another sender had the same destination and killed the receiver)
2. events it sent could have never had a chance to process, even if it was generated by a "fast" Actor.
3. Which ever piece owned the sender of the first event to be handled always has the advantage.
4. EN is ill suited for thousands of events being all generated at once. EN is more suited to a fluid environment (things happening all the time) IMO.

Regards
-A

[TheIdeasMan]

Yes. Glad you did. I modified the old code to accept the new events.

So you didn't write the basic code I asked for? My aim was to start out with something very simple (seemingly pathetic even), and gradually add complexity. You haven't done that, now we have still have the problem we had right at the start: Code that doesn't work as well as it should.

It could be an int in the Gamepiece class that represents how far it is from being destroyed.

To me, Gamepiece and Actor might be conceptually the same thing. As for Actor being an int, don't confuse that with the concept of a Health value/object. Actor is also a type required in containers and functions, described in more detail below. In a simple scenario Health could be a member variable, in a more complex situation it could be it's own class because it is integral to the Combat class operation. Representing everything with int is a criticism I have about your original code. IMO there are valid places for it, in particular using enums, but one should be careful not to overdo it.

Why do we have Player and Enemy classes?

In addition, we will need containers and functions which will be declared to take pointers of these types. In a polymorphic fashion, these will be filled/called with pointers to derived classes. The same applies to the Weapon class or any Resource class. For example, a Player might have multiple weapons, so this could be a std::vector<Weapon *> Also for events, for example in the Subscription constructor.

Why is Combat a member of Actor?

In addition, we don't want to pollute other classes with this stuff. And because we want it to be inherited, so it isn't repeated in a bunch of other classes.

Regarding Event Notifier:
How I wanted to use it was instantaneously generate 99 % of the events.

That doesn't make any sense to me at all, and is contradictory to the purpose of an EN. It doesn't fit with what would happen in the real world: A battle doesn't happen all at once, it's sequential thing. Why do you want to do this? I suspect you still have a data driven mindset.

When there was a new death event, in a separate container, remove all events where it was either the sender or receiver.

That confirms my diagnosis of a data driven mindset (again). A death event should be broadcast to all objects who were shooting at it. Those objects remove that target from their own target list (a member variable). No huge external vector or table of who shooting at who, and whether something is dead or not.

1. events directed towards it could have been wasted (a sender basically did nothing if another sender had the same destination and killed the receiver)

What's wrong with that? I shoot at a target, not realising someone else killed the same target 0.1 seconds before. An obvious consequence of multiple shooters at one target. As long as no one is repeatedly shooting a dead target.

2. events it sent could have never had a chance to process, even if it was generated by a "fast" Actor.

The EN should be asynchronous - it works in it's own thread or threads. This helps prevent everything else being blocked while the EN processes the events it has.

3. Which ever piece owned the sender of the first event to be handled always has the advantage.

That sounds like a consequence of the data driven methodology. Although whoever sends the first event will have the first entry in the EventHandler, but that is fair enough: it happened earlier than anyone else's event. Remember that event is removed from the EventHandler once it is processed.

4. EN is ill suited for thousands of events being all generated at once. EN is more suited to a fluid environment (things happening all the time) IMO.

The EN should be able to handle at least thousands of events per second if it is threaded. Again, I don't get your concept of "all generated at once".

[a10e29]

So you didn't write the basic code I asked for?

Here you go. Didn't include the GamePiece side of things because it's being redesigned. more on that later.

bool Soldier::duel(Actor* target, bool callAgain)
{
	for (int i = 0; i < getMyData()->getMyCombatSkills()->numberOfAttacks; i++)
	{
		if (getMyFight()->targetWasHit())
		{
			if (!target->getMyFight()->attackWasBlocked())
			{
				target->getMyHealth()->hurt(getMyFight()->getMyWeapon()->getDamageDone(getMyData()->getMyCombatSkills()->iniative));
				if (target->getMyHealth()->getCurrentHealth() <= 0)
					return true;
			}
		}
	}
	if (callAgain)
	{
		return target->duel(this, false);
	}
	else return false;
}

It's not great, and heavily influenced from the old code, but it is two way interaction. Probably not taking advantage of the combat class as it should.

Representing everything with int is a criticism I have about your original code. IMO there are valid places for it, in particular using enums, but one should be careful not to overdo it.

Agreed

In addition,

Good additions, thanks

It doesn't fit with what would happen in the real world: A battle doesn't happen all at once, it's sequential thing. Why do you want to do this?

What's wrong with that? I shoot at a target, not realising someone else killed the same target 0.1 seconds before.

I'm using a concept of time that isn't measured in seconds, minutes, etc. it is measured in turns, each turn representing x hours/days/years or whatever.

Again, I don't get your concept of "all generated at once".

All events are pushed onto a queue before they begin to be processed.

The EN should be asynchronous - it works in it's own thread or threads. This helps prevent everything else being blocked while the EN processes the events it has.

That sounds great. No experience with threads though. I'll start a mini side project to play with them and direct any questions on them to the general forum.

I suspect you still have a data driven mindset.

That confirms my diagnosis of a data driven mindset (again)

Just don't accuse me of knowing C. I probably know more about Java. For example, when you were getting your appendix out, I learned that char foo[50] is a c-string. I honestly thought it was just a c-style array of char.

As I implied in the beginning of this post I'm redesigning GamePiece, it probably has more than it should and I've removed 6 functions already (placed into ActorManager) I've annotated it with where things should go. What I'm looking for:
1. Points you disagree on
2. Things are aptly named

class GamePiece
{
	// LM = locationManager
	//GPCombat = GamePieceCombat
public:
	GamePiece();
	GamePiece(int, int, Hex*, SDLToolBox*);
	void moveTo(Hex*); //  should stay
	void wasSelected(); // fills targets and moves
	void clearMovesAndTargets(); // should stay but use LM and GPCombat
	void render(); // should stay
	void turnStartReset(); // should stay
	Hex* getCurrentHex(); // to LM
	int getActions(); // should stay
	int getRange(); // to GPcombat
	virtual void attack(GamePiece*) = 0; // use GPCombat
private:
	Hex* currentHex; // to LM
	std::vector<Hex*> targetLocations; // to GPCombat 
	std::vector<Hex*> possibleMoves; // to LM
	std::unique_ptr<RenderData> myRenderData; // simple struct with it's apportiate render box and texture
	std::unique_ptr<ActorManager> actorManager; // just added 
	int actions; // should stay
	bool left; // toLM
	SDLToolBox* tool; // should stay
	void checkToFaceLeft(Hex*); // to LM
	
	void targetSearch(int, Hex*); // to GPCombat
	void fillPossibleMoves(int, Hex*); // toLM
	bool inCanMoveTo(Hex*); // to LM
	bool inTargets(Hex*); // to GPCombat
	
};

[TheIdeasMan]

Hi,

We still seem to be at cross purposes here. We all (the other respondents at the start) wanted you to abandon your original code because it was a mess. I wanted to have you start very simple and build up. But you still seem intent on modifying the original code while attempting to incorporate (IMO badly) some of the ideas I suggested. Even the "dead simple" code is way over complicated, and still doesn't demonstrate understanding of the OO principles.

I have been wondering for awhile whether this whole thing is an elaborate troll. Is that too harsh? Think about it from my point of view: it's easy to come to the conclusion that the discussion is being run around in circles with extra complications and distractions, very little of what I have suggested is being implemented. Mainly that advice about keeping things simple is being ignored. Realise that a complex project can be built from simple things.

Can you show cause as to why I shouldn't be thinking like this? Can you see how I might be wasting my time?

Some examples of why I think you aren't yet thinking OO:

bool Soldier::duel(Actor* target, bool callAgain)

Actor* target So a Soldier can shoot at other soldiers, and at itself? This misses the point of inheritance: A soldier shoots at Enemy, while Enemy shoots at Soldier. The parameters should reflect that.

Not simple:

target->getMyHealth()->hurt(getMyFight()->getMyWeapon()->getDamageDone(getMyData()->getMyCombatSkills()->iniative));

This is the exact sort of mess we are trying to avoid. I had in mind Soldier::Shoot(Enemy *) and Enemy with bool IsDead() and ApplyDamage(const std::size_t DamageArg) Really simple, pathetic even. Can you see I am trying to start with simple but clean code, then build on that - not build on something that is small but has inherent mess, which quickly turns into big mess? Sorry if you think I am treating you like a ten year old, but until you are cognisant of these OO principles, and you learn to abandon your original code, nothing is going to change.

It's not great, and heavily influenced from the old code, but it is two way interaction. Probably not taking advantage of the combat class as it should.

When are going to realise that I have no interest at all in your original code - or anything that looks like it?

Just don't accuse me of knowing C.

Far from that, trying to show OOP. Data driven isn't exclusive to C, but mixing C paradigm concepts into OO code could be considered bad form. I am not saying that static or dynamic tables are bad news - there are places where their use is appropriate, but doing that instead of having OO objects look after themselves and interact properly is wrong IMO. Quite frankly that's what might lead to messy code.

I probably know more about Java.

Why couldn't you implement the EN properly then? As in trying to do it with less classes than what they had?

For example, when you were getting your appendix out, I learned that char foo[50] is a c-string. I honestly thought it was just a c-style array of char.

It is just a c-style array of char. The concept of a c-string comes from the idea that it is null terminated, so printf can take a char array argument and print it without having to know it's size. It's null terminated when created from a quoted literal, otherwise one has to put in the null manually.

What I'm looking for:
1. Points you disagree on

All of it.

You now have: std::unique_ptr<ActorManager> actorManager; another example of what I am talking about - just another ill defined curve ball. Just adding more (probably ill conceived) stuff to the original messy code, with no regard to OO at all.