Home » Stars! Clones, Extensions, Modding » Stars! Nova - Development » Item.Mass - should this be an abstract property?
Item.Mass - should this be an abstract property? |
Tue, 07 February 2012 14:51 |
|
ekolis | | | Messages: 51
Registered: May 2006 Location: Cincinnati, OH, USA |
|
|
As I was looking through the code, I found that lots of things inherit from the Item class, which has a public Mass field. This includes fleets and fleet intel, which makes the field somewhat problematic. After all, the mass of a fleet is a computed value - the sum of the masses of all constituent ships. But this is never initialized, so fleets and fleet intel have zero mass! This is even handled inconsistently in the GUI: a fleet report shows its mass as zero, but the "my fleets" list shows the appropriate mass, so the calculation is being done - it's just being done inconsistently.
I think this could be solved by making Item's Mass field an abstract property instead. Then ships could implement it by checking their design, and designs could implement it by summing the masses of their components, and fleets could implement it by summing the masses of their ships.
If this causes performance issues, the values could always be lazy-loaded and cached, but I doubt this will be a problem to begin with.
Also, Item has a number of other public fields which I haven't looked into, but I suspect some of them might warrant similar refactoring.
Thoughts? Concerns?
Mr. Flibble says...
Game over, boys!Report message to a moderator
|
|
| |
Re: Item.Mass - should this be an abstract property? |
Fri, 10 February 2012 15:05 |
|
ekolis | | | Messages: 51
Registered: May 2006 Location: Cincinnati, OH, USA |
|
|
Hmm, this doesn't look like it's going to work... Some Item subclasses should have writable mass, cost, etc. but others should not. Thus I'm faced with a dilemma: If I declare the properties read/write, then I'd have to throw exceptions in some of the setters, but if I declare them read-only, it would be impossible to set them when necessary (e.g. for components and hulls)! I can't even declare the property {get; protected set;} and then make the setter public on the appropriate subclasses! This could be worked around using interfaces somehow, but I'm not sure it's worth the trouble...
edit: Actually, I've got another idea... Why are all these things in Item anyway, if they don't apply equally to all items?! Maybe they should be pushed down to the appropriate subclasses... not sure exactly how that would work at the moment though!
[Updated on: Fri, 10 February 2012 15:08]
Mr. Flibble says...
Game over, boys!Report message to a moderator
|
|
| | | | |
Re: Item.Mass - should this be an abstract property? |
Sun, 12 February 2012 02:28 |
|
Aeglos | | | Messages: 142
Registered: May 2011 Location: Chile | |
|
I agree, but for example; Ship inherits Item, and contains a Design which ALSO inherits from Item. That's pretty redundant.
I'd rather have Ship inherit a Design directly so there's a clear line of inheritance.
And as stated somewhere in the code in an old comment, Item should inherit from design, and not the other way around. It makes no sense for a design to have mass and position; an Item is a much more "concrete" thing anyway, so it should logicaly be lower down in the implementation chain than a design.
Although I also concede that this kind of logic not always makes sense in software, and I don't know what implications reversing the inheritance might have on the codebase other than the Key properties we worked so hard to implement would need to be realocated to track designs by Id.
Design -> ShipDesign -> Item -> Ship
Would be an ideal arrangement, instead of the current:
Item -> Ship
|->Item -> Design -> Shipdesign
Even
Item -> Design -> Shipdesign -> Ship
Would be cleaner, and it could be a simpler change to implement.
[Updated on: Sun, 12 February 2012 02:30] Report message to a moderator
|
|
|
Re: Item.Mass - should this be an abstract property? |
Sun, 12 February 2012 15:50 |
|
ekolis | | | Messages: 51
Registered: May 2006 Location: Cincinnati, OH, USA |
|
|
Aeglos, I think you're confusing inheritance and composition. Ship should not inherit from design, as it makes no sense to say "a ship is a design". Rather, it is correct to say "a ship has a design". Thus the current system of ship object having a reference to a design object.
I think the main problem is that the inheritance hierarchy is too top-heavy: Item has way too much information that is irrelevant for certain subclasses.
I agree with Daniel that we should think this out, though.
Oh, and by the way, I'm not exactly a "new" developer on the project, but neither am I all that experienced with it - I was involved with Nova a bit several years ago, but then I got involved with another project, called Star Legacy, so I left the Nova project. Unfortunately Star Legacy has kind of fizzled out, but on the bright side, I can get involved with Nova again!
Mr. Flibble says...
Game over, boys!Report message to a moderator
|
|
| | | | |
Re: Item.Mass - should this be an abstract property? |
Tue, 14 February 2012 13:32 |
|
Daniel | | | Messages: 179
Registered: April 2006 Location: Nowra, Australia | |
|
I am thinking of something like this: https://sourceforge.net/apps/mediawiki/stars-nova/index.php? title=File:Nova_game_object_proposal.png
I have left out all the component/component properties as that seems fine to me (but then, I built that.)
I am also not concerned with Hull, HullModule or StarList.
That leaves the current objects: Item, Design, ShipDesign, Ship, Fleet, Star, MineField.
I think we need to keep Star, ShipDesign, and Minefield in much the same form they are currently.
I think that Fleet could take on a couple of extra properties to track shield and armour damage in order to get rid of Ship. Damage is not managed per ship in Stars! However an argument could be made for keeping Ship in order to allow for future expansion. In that case it would descend from GameObject.
I have retained Design, however I put a question mark over it because the only things that use design appart from ShipDesign are planetary installations. Mass and key are not really relevant to these and the planetary installations could be refactored as properties of Star.
I have split Item into GameObject and MappableObject. This removes the Mass property from Star, Fleet and MineField. Whilst all these things could have a mass, it is irrelevant for Star and MineField and is calculated for Fleet (can not be set). This scheme also removes position from those objects which are not tracked on the map.
Ideas/recommendations?
Have fun.Report message to a moderator
|
|
|
Re: Item.Mass - should this be an abstract property? |
Tue, 14 February 2012 15:28 |
|
ekolis | | | Messages: 51
Registered: May 2006 Location: Cincinnati, OH, USA |
|
|
Funny you came up with GameObject; that's what we called our base class in Star Legacy as well!
At first I was a bit confused by the idea of getting rid of Ship - how would you ever have ships in the game without a Ship class?! But I quickly realized that since Stars! doesn't track damage to individual ships, but instead tracks damage to "ship stacks" similarly to the original Master of Orion, there's no need to instantiate an object for every single ship; all you need to do is keep something like a Dictionary<Design, Tuple<int, int>> to tell you how many ships you have in each stack and how much damage each stack has taken!
One question, though - why does GameObject need a Type property? Can't you tell what an object is by using the built-in GetType method of System.Object, or the "is" operator? Or is this something more detailed than just the CLR-type of the object?
Mr. Flibble says...
Game over, boys!Report message to a moderator
|
|
| | | | | |
Re: Item.Mass - should this be an abstract property? |
Tue, 13 March 2012 18:39 |
|
Aeglos | | | Messages: 142
Registered: May 2011 Location: Chile | |
|
Indeed, on better examination it looks like not even ShipDesign might need mass/cost, as it can derive them from the sum of it's hull/components.
I'll implement a "buildableobject" (name pending, I suck at naming classes) and remove component from the current inheritance altogether.
EDIT-
Ok. I moved Mass and Cost to Component instead of introducing a middle object. They are now responsible for keeping that data. Other classes like Fleet or ShipDesign still have properties to access Mass and Cost information, but they are just calculated properties;
i.e. ShipDesign.Cost just returns the sum of it's component's costs, Fleet.Mass returns the sum of it's token's masses, etc.
Since Fleet/ShipDesign/Whatever don't need to share an interface or be polymorphic with Components, I think this is the cleanest way to improve the design.
Component doesn't need a Key, but it needs the Name that Item provides. I'd leave it as it is, as I think it would be nice to change the components to actually USE the Key field in the future (not critical though)... not really a fan of indexing by string name.
[Updated on: Wed, 14 March 2012 01:04] Report message to a moderator
|
|
|
Goto Forum:
Current Time: Thu Apr 18 22:46:59 EDT 2024
|