Moving from JS to C# - Change in code architecture?

I've decided to drop UnityScript and start learning C#.

I've got a project I've been working on that I'd now like to convert to C#, but I have some questions about the overall architecture of the code. C# seems to have better options and coding patterns available, including support for namespaces, which I can already see as becoming a nightmare in UnityScript.

Here's an overview of a turret and weapon component that I have working completely in JS.
What I'd like to know, is if there is a better way to implement this now that I'm transitioning to C#.


Here is a turret that can be controlled by the AI or a player. Each box is a separate script.

Turret: Does the actual aiming of the turret. It will acquire a target value from either Targeting or Player, and it will attempt to aim at it.

Weapon: Holds all the weapons properties. The turret might have more than one weapon.

FireControl: Bridges communication between the turret and the weapons (if any). If under AI control, it asks the turret if it is aimed at the target, and if the weapon is ready to fire. If both are true, it sends a shoot command to the weapon.

Scanner: Searches for valid targets, and sends them to a Dict held in Targeting.

Targeting: Iterates through the target list looking for the best target according to one or more parameters. ( Closest, most damage, largest, threat, etc. )

Player: If under player control, Turret will use an aim point as the target, and ignores the target in Targeting. FireControl also attempts to fire when the player clicks, as opposed to aim.

The scripts are split up so that I can swap out different components with ease.
Different weapons can be used, as long as they implement the things FireControl is looking for.
I can provide different scanner functionality, or multiple scanners, all contributing to the target list.
Or even make a turret with different movement features and it won't affect the weapon or targeting.

So, how does this look? Am I going to run into problems down the road?
Is there a better way to set this up? Does C# provide a way to do this that I haven't considered?

 

sounds like a classic "insert interface here". https://unity3d.com/learn/tutorials/modules/intermediate/scripting/interfaces

it's a feature available in unityscript and c#, but if you haven't been using them it might help ensure the relevant components employ the necessary methods that other components depend on

 

Looks good to me.

Since UnityScript is basically C# dressed up to look like JavaScript (worst idea ever), there isn't any real differences in how you'd do things. Namespaces are not that powerfull in C#, as they're simply shortcuts for longer names, but they're nice to have around to keep the size of your autocomplete-lists shorter.

The only thing I can think of that might become an issue is this:

If you haven't already done it, having an abstract FireControll class that you subclass into an AIFireControll and a PlayerFireControl might make things be a lot more manageable than if you're doing something like

if(controlled_by_ai) {
...
}
else {
...
}

The same thing goes for weapon - having public variables for things like firerate and turnrate and the bullet prefab will do for some differentiation, but if you want different weapons that have fundamentally different properties (parabolic firearcs, flak shots, etc.), you'll want to subclass that out too.

This isn't a C# consideration, this is just a programming consideration. Your script setup looks good.

 

Looks good to me. Structure is generally language independent.

Contrary to @Baste, I will make a plug for namespaces. Namespaces are a great way to separate systems and control dependencies.

 

if you want to learn c# better dont do it inside unity, cause unity makes many things to easy and you will have stuff running without knowing what it exactly do

 

Depends on your goal. If you are learning C# for its own sake, then sure, step out of Unity. If you only want to make games, there is actually no reason to leave Unity.

 

Yeah, I already felt a little uncomfortable about the way Player, FireControl, and the AI are laid out.
Player seems to be a little shoehorned in, and that's because it is. Originally, this was a project for AI control of turrets. And it shows.

@Baste, I think you're on the right track.
Control should be a new abstract class with Player and Targeting inheriting from it. And Turret looks to them for their target variable.

FireControl keeps a list of the weapons on the turret, which one is next to fire (in the case of alternating fire, for example), and could even keep track of expected damage output, so that Targeting can choose not to waste ammo on overkill.
So, while it contains some info for the AI decision making process, I don't think it makes sense for it to be a base class for Player and Targeting. Since it also does things like slightly angling guns if they are to converge fire at a particular range, and which weapon is next, as already mentioned.
But maybe there's a better way to implement FireControl structurally.

But now I have a question:
If Control is a base class, and a turret might have a Player or Targeting script (or both), how do I get Turret to know which one to look at for the target? Does it have to test for both during Start()? If I add a third type, do I have to then test for that too? (That sounds like it could get unwieldy) Or should the subclass of Control push the target into Turret?

 

Use an interface or a abstract base class. That way the Target is always looking for the same type.

Another thing to consider is the direction of dependence. Typically I would have this reversed. The various AI classes push the target down the chain.

 

Well an abstract class can't be instantiated, so I'll have to push the target into Turret, right?
I mean, since Turret doesn't know whether to look for Player or AI, so it can't grab the value. Right?

 

Polymorphism. While an abstract class can't be instantiated, one of it's derived classes can. And the magic of OOP is that any derived classes are the base class. You can simply have Target look for the base class, and not know about the derived classes.

Same principle works with interfaces.

 

target is a variable in the base class Control.
How is Turret going to reference target in the derived classes of Targeting or Player? Can you show me in code?

 

Pseudo code follows. I'm on my phone, so there may be a couple of compiler errors.

Code (CSharp):

1. public abstract class BaseAI : MonoBehaviour{
2.     public Transform Target;
3. }
4.  
5. public class Human : BaseAI {
6.     // Some code to set target
7. }
8.  
9. public class Targeting : BaseAI {
10.     // Some code to set target
11. }
12.  
13. public class Target : MonoBehaviour {
14.     // In some relevant method
15.     BaseAI baseAI = (BaseAI) GetComponent(TypeOf(BaseAI));
16.     baseAI.target // do something useful
17. }

 

Ah... TypeOf(BaseAI) is the critical part that I was missing. I hadn't realized you could do that.

 

GetComponent<BaseAI>() frees you from doing the cast. It didn't work in Unity 4 (you could only use classes deriving from component in the variable GetComponent), but it's all fine and dandy in 5.

 

Nice. I'm guilty of missing all of these little API updates. I keep on top of the major ones, but some of the small ones slide past me.

 

Ok! So I have a proof of concept working.

Abstract class AbstractTargeter ( has target variable )
Abstract class AbstractTargetList inherits AbstractTargeter ( has targetList dictionary )

Then targetList is populated via:
class Scanner
or
class AdvancedScanner

Then target choosing is done via:
class BasicTargeting inherits AbstractTargetList
or
class AdvancedTargeting inherits AbstractTargetList
or
class Player inherits AbstractTargeter (because it doesn't need the scanner or dictionary)

What I'm wondering is, if it would be better for AbstractTargetList to be an interface instead of using a couple levels of inheritance.

I originally was thinking BasicTargeting would inherit both AbstractTargeter and AbstractTargetList, but found that C# doesn't do multiple inheritance like that. So I chained the inheritance, which provided what I needed, but it feels a little like a workaround.

The issue I'm worried about, is that targetList is a dictionary that holds a TargetData class.
If a more advanced scanner is used, more target properties will need to be recorded into a more comprehensive class, such as MoreTargetData, which will then inherit TargetData.

This will probably mean that I'd have to replace AbstractTargetList with something that can hold a dictionary with MoreTargetData. It would still have to inherit AbstractTargetList so Player/Basic/Advanced/Targeting can see it, but I don't think I can replace/extend the older dictionary like that.

Does interfaces solve this? Did that make any sense?

 

Here are a couple of thoughts:

Code (csharp):

1. class AbstractTargetList : AbstractTargeter

Just based on the names I don't think this makes sense. If something is a list of something else, I would expect it to contain many of those item's, not subclass them. Reading into your design, there's no need to create a new subclass for every additional piece of data.

Code (csharp):

1. class Player : AbstractTargeter

This makes even less sense to me and it violates the Liskov Substitution Principle. Ask yourself this: is a really player an abstract targeter? Don't forget that C# doesn't allow multiple inheritance, so you have trapped yourself with a class called Player that might do some targeting but can't easily do anything else.

What I think: the Player doesn't need to be a Targeter because it can choose it's target with a "targetList". I'm guessing here, but they player probably chooses a target by clicking on them. So why does it need a list? It can see the targets on the screen. I also think it's a mistake to have Turret know about Targeter. The turret shouldn't care how it is getting directions, just that it has a directive to aim at something. Give the turret an interface which allows setting a location to aim at, then give the AI a Targeter to help it decide which location to tell the turret to aim at.

I don't think targetList should exist outside of the class that is doing the targeting. There is no need to dictate how a scanner should keep track of it's targets. Maybe I want to use a hashed set, or just pick targets on the fly.

Just a point of interest since I don't think you should go this route: if targetList is a List of type TargetData and you create a subclass of TargetData called MoreTargetData, targetList will be capable of holding MoreTargetData objects. This is because a List is a type of variant called a source and sources are covariant on their type.

I'm interested why you think the targetList is important to the Player. I think the list of targets should be an internal concern for which ever object is making the target choice. A player should be allowed to choose any target they can see, while the AI will need some help determining which targets are valid. The AI, then, can have a reference to a Scanner component which does the first job of finding all potential targets. Then the Targeter component can accept this list as an input and apply whichever rules it sees fit to choose only one target.

Additionally, I don't think Player needs to know about Turret - This class can tell the FireControl class who they would like to have the turret aim at, thus removing a dependency on Turret.

I attached a class diagram.

 

Thanks for the in-depth analysis.

A lot of the structure is a result of operating under the idea that an individual script will grab everything it needs as opposed to other scripts pushing things to it. This made the abstract classes necessary so that grabbing values would have a non-changing class name (base class) even if the class that generated the values changed.

This could be flawed thinking, however.

I think it needs to be separate because there may be multiple scanner types contributing to the targetList, and there may be multiple targeting classes and of different types. I think it makes more sense for the scanners to push and manipulate the targets on the list, as the list has no clean way of knowing where and how many scanners there may be. Similarly, there may be multiple targeting classes accessing the targetList (because of multiple turrets) and they have to access the list on their own. Again, because the targetList has no good way of knowing what classes are looking at it.

Well, originally, I didn't have targetList associated with player. I'm considering it now, because it determines what a unit can see, and thus what the player can see.

But it might be better to set up a separate class to handle visibility for players.

That looks like a nifty tool. What is it?

 

I prefer designs that work in the other direction as I find they are easier to refactor to support dependency inversion once they become more complex. Put another way: I prefer one directional dependencies over two directional and since the high level objects must have a dependency on low level objects in order to use them, I try to eliminate any possibility a low level object will need upward dependencies. Since a high level object has a better "view" of the design and is able to reach into more objects without breaking my no-upward-dependency rule, they are in a better position to collect the references needed by the low level objects.

This is interesting. What exactly does the list represent? Why would you need more than one scanner to fill it?

This is related to my previous question .. If there is a good reason to have multiple scanners then a different design might be needed. Anyways, when an object uses a field or method on another object it is said to have feature envy; the Scanner class would really like to have ownership of the list.

From your design it looks like the Scanner and Targeter classes would like to have ownership of this list but as far as I can tell, the Turret class doesn't even care about it. For this reason, I would definitely not keep the list on Turret. The question becomes move it to Scanner or Targeter? I prefer putting the member on the class which sets the field's value most and give less importance to classes which just want to get the value. I have two reasons:

1. When an object wants read and write access to another's fields, it must know about two methods - the getter and the setter. If it just wants to read the field, it only needs to know about the getter method. This leads to a more segregated interface - which is a good thing. And indeed, the .net framework provides a great implementation of this segregated interface. It is called IEnumerable<T>, and it is very, very simple.
2. Particular to variants, which is what any collection is, getter methods are covariant - that means they share the same direction of inheritance as the types which they are variant upon. Setter methods, on the other hand, are contravariant - that means they have the opposite direction of inheritance as their underlying type. This makes setter methods more complex, and I prefer to encapsulate that as much as possible.

Personally, I think you can just treat the Scanner as a list instead of maintaining one as a field. It's also not really the interest of the Turret to know about the list or even the Scanner or Targeter. That would be the job of the AI.

For the player, you may need a special type of Scanner to determine valid targets but I think this list already implicitly exists: it is the set of enemy units that the player can see on the screen. This is probably controlled by the enemy units themselves along with the cameras rendering rules. The enemy's position, and possibly their ability to cloak, along with other rules like fog of war and camera frustum would have an effect on how they are rendered by the game.

It's called NClass, it's a really simple program for making class diagrams. There's not much automation so it requires a functioning knowledge of UML and there isn't a lot of functionality outside of making the lines straight, which is a real boon for the artistically challenged like myself.

 

I'll describe a potential scenario:

targetList is all the hostile targets a particular unit knows about.
Mainly, this information will come from a unit's own scanners. However, some info could conceivably come from an allied unit as well - letting other units on its team know about targets.

A unit may have more than one scanner because target detection is broken down into two separate phases:
1) Every few seconds, a long range detector gathers general target info and puts it on the list. This check needs to run GetComponent() on potentially many units, so for performance reasons, it's best that this runs only once every few seconds. This represents the detector, a type of scanner.
2) Much more frequently, a short range analyzer iterates through the list, checking if any of those targets have moved into analyzer range. If any have, it will collect more detailed information about them, typically for weapons and tactical decisions. The analyzer is another type of scanner.

Then there is the possibility that a unit might have very directional scanners limited to a small area of the units view. As would be the case for a very long range directional analyzer or detector, that supplements its more conventional scanners.

Or that there might be a self-contained point defense turret that comes with its own short range analyzer on a rapid refresh rate. It needs a rapid update rate to be able to catch very fast moving targets. Giving it its own target list could become prohibitive for performance reasons, so this fast analyzer supplements the other scanners.

So that describes the multiple scanners aspect.
But there may also be multiple targeters.

Consider that a unit may have multiple turrets, each with a different target priority.
But there's no need to give each of them their own targetList, since that info has already been gathered.
So the unit has an overall AI, and then individual turrets have their own simpler AI.

An example:
Consider a WWII battleship. It has multiple detection systems, crude radar, sonar, and human eyeballs. It also has spotters radioing in various long range targets. It has large main gun turrets that will fire at big, distant targets. And short range machine guns to fire at enemy planes.

On a separate note, going back to inheritance:
If different sets of classes need to look at targetList, I'm having a little trouble seeing how I'll be able to extend TargetData, without simply redefining it at the application level, which is probably not ideal.
A scanner might know it needs to use MoreTargetData, for example. But a targeter will still be looking at the base class, not knowing that it has been extended.

 

Yikes. This is getting pretty sophisticated.. I don't want to design a WWII battleship over a forum.

I think it is important to recognize a computer program does not simulate the real world perfectly and you must be careful to create relationships that make sense from a computer's point of view rather than a human's.

Sometimes we hold metaphors for programming concepts, comparing them to real life things. This is normal since it is very difficult to think like a computer. However, we need to remember that they are just metaphors.

It is okay to think of an array as a place to keep a collection of things. However, I think there is a danger in stretching it's use too far, such as like a tool for sharing information. Object oriented programming can help us a bit here by defining classes which are more sophisticated that simple arrays. However, there will still be a point when the metaphor we hold for this advanced class breaks down and it does not behave the way an object in real life would.

If you want to have multiple classes communicating with each other, they should have an interface for doing that. I don't think all writing to the same array, which is held in another class, is a good approach.

This seems to be at odds with your original design in which each turret contains a targetList. Anyways, even if you are to change the design so that each of the smaller components on the larger can use the same targetList, it would be a mistake to have the smaller components inherit from the unit as a whole just so they can have access to the targetList. A better solution is to simply have the unit communicate it's knowledge by passing the list of items to each of it's components.

It is very easy to apply polymorphism in places it does not belong. I think it is most common to see what appear to be polymorphic relationships between domain objects. However, just because they are similar does not mean that they are the same thing. The Liskov Substitution Principle tells us that in order for an item to make a good subclass, it should behave exactly as the base item does in every situation the base item is used.

 

This is stemming from your desire to have many types of scanners and targeters working off the same targetList. I don't know exactly how the code is working in your head but I envisioned the scanners as simply detecting GameObjects that were within a certain vicinity. Once the GameObject has been found, a targeter is free to access as much information from it as it likes. It does not need to rely on the scanner to identify the GameObjects type of weapon or armour. It simply needs to ask the GameObject for the value of its public properties.

If you decide to create a kind of "target meta data" to simulate a real threat detection system such that the targeter must rely on the scanner's ability to collect data, you could use a factory method to create the correct type of targeter based on what kind of data you have. If your scanner collected AdvancedTargetingInformation then create an AdvancedTargeter. If, however, you only have CrappyTargetingInformation, create a BestEffortTargeter.

 

Well, I've already got everything working. (It's also a space ship, but a battleship seemed to be a clearer example)

What I'm trying to do now is redo the code to be better and more versatile. Particularly, so I can package the parts and reuse them in other projects, and even put some on the asset store.

Also, the turrets have never contained the target list. There may have been some miscommunication along the way. Right now, it's part of the detector class and I'm trying to see how I can split it out for better versatility.

I'll take a look at factories. I really appreciate the insight you've provided.

 

I've only been vaguely following this thread, but it seems to me like you need a layer between your scanners and your turrets.

So it would look something like this:

• Scanner: responsible to locate targets
• TargetHolder: responsible to keep a collection of all targets. Can either check all available scanners for targets, or can wait for targets to be pushed, depending on which direction makes sense for dependency.
• Turret: responsible to fire on targets. Again can pull a target or be pushed to, as makes sense for dependency.

 

Haha.. I'm not sure if that was intended to make me feel better ..

Yes, I was basing that assumption on your original diagram, though I see now there is no "targetList", just "target".

Another option you may consider is the Chain of Responsibility pattern. You could have a chain of targeters which start with the most sophisticated and end with simpler ones. When the chain receives a targetList, the first targeter would decide if the target information was detailed enough for its algorithms. If there was not enough information, it would pass the list to the next targeter in the chain and so on. Hopefully, one of the targeters will accept the targeting information and make a decision. Otherwise, you could have a default target choice, such as the first in the list.