Re: [scorch-devel] Re: SE2K: Weapons and refactorings

[Mikhail K. <me...@cs...>]

> > Impossible, unfortunately. It can't be static, because it than would be 
> > shared by all instances of the game running in the same JVM, which can 
> > happen if somebody opens up applet in two windows of the same browser. 
> > This would screw up our synchronization based on the same random # 
> > sequence. You could make it non-static publically available object in 
> > ScorchApplet, or ScorchField, but I think I decided again it because that 
> > meant that I would have to pass reference to that containing object 
> > everywhere. 
> > 
> 
> Ah... didn't know applets would do that.  I also didn't know it tied 
> some how into your
> synchronization.  I thought it was more for just getting a random 
> number.  If you don't mind me
> asking, at which class(es) does the synch issues come up for this 
> Random number?

It's not synchronization in the sense of synchronized (rand) { ... }
What happens is that when games starts, server generates a seed. It sends 
it to all clients and they initialize random number generators with this 
seed. Then it is important that every client generates exactly the same 
number of random numbers in exactly the same order. That's how we make 
sure that all clients have the same terrain generated etc.

> > Wait, the only thing that weapon knows about the extending classes are 
> > their names. It's easy to get rid of this: we can use reflection to list 
> > all classes in "scorch.weapons" package which extend Weapon. Then call 
> > getName() on them etc. I'm not sure it's worth the effort. Factory is a 
> > possibility too. If you feel like it, try it. Of course changes to Items 
> > should be symmetrical.
> > 
> 
> Interesting idea.  Might work well.  Sort of has a Module plug-n-play feel.

It might work, might not. There are some issues in Netscape Communicator 
with security and reflection. I don't want to break NC support just for 
that. But if it works it would be quite cute feature -- add weapons 
without recompiling. Useless, of course, in an open source project. But on 
the other hand you will be able to *remove* weapons without recompiling.
 
> > Yes and no. Explosions are not eactly the same as weapons. I guess the 
> > right way to do it is to add a flag to each weapon which would say whether 
> > it can be used for the death sequence. I need to think more about this, 
> > but just moving this to weapon or weapon factory doesn't sound 100% good.
> > Much biggre problems is with Items. ScorchPlayer has a lot of knowledge 
> > about specific item classes.
> > 
> 
> I like the idea of having each weapon know if it can be used in a death 
> sequence.  

Please don't call it weapon, it's explosion. Otherwise it's confusing. 
There might be some problem lurking in there. Ah, I know. All the 
explosion classes take slightly different arguments so to generalize that 
will have to path a bunch of useless arguments to some explosions. For 
example only FireExplosion needs height of tank, only Fire and Laser need 
width of tank and only FunkyExplosion needs rand object.

> I guess I need
> to look at the Items and ScorchPlayer ties more closely.

I though about that a lot. I don't think this tie can be easily broken, 
because unlike weapons, items influence things in environment a lot. 
Things like tracers, parachutes, auto defense can not be self-contained. 
This is a case when design and incapsulation must clearly be sacrificed 
for practicality. 
 

> > I think I have a better idea. I kind of like the fact that 
> > produceExplosion is so generic and that weapon class doesn't have to 
> > implement any methods. The solution I have in mind is to make argument an 
> > Object isntead of int. For example MIRVExplosion needs really 2 arguments: 
> > number of particles and radius. Then the object passed to is as arument 
> > would be an array of two ints. This might introduce some weird relation 
> > ship between Weapon and Explosion class, but it looks good to me. At least 
> > at 3am. Let me know what you think about this idea.
> > 
> 
> Yes... I was thinking of changing the single attribute int to a array 
> or hashmap.  If it was a hashmap, you could do a name value pair.  In 
> either case, I still wasn't sure if there was a need
> to hide the Explosion class.  Here's the benifits/tradeoffs that I see:
> 1) current way - requires less code, increased complexity in the 
> explosion class being called

There is one more benefit: I was thinking of completely getting rid of 
Weapons classes and just turning them into a text file with weapon 
properties. This way people would be able to tweak weapons without 
recompiling. With the current scheme this change is more or less trivial.

> 2) using abstracts - forces developer to implement the child class 
> correctly, more obvious what is
> going on

I'd prefer to stick with 1) really. I think it will look much better if we 
change arg into an Object and stick arrays into it when possible. I don't 
want it to be a Hashtable because in most cases there will be just one 
argument, sometimes two. Hashtable seems to be an overkill, even though 
the idea of having names does sound good. 
I just think that changing stuff which is not broken is almost always a 
bad idea. It's ok to do when the benefit for design/readabilitiy is 
obvious, but when in doubt, leave the lying dogs lie.
 
> Ok.  let me know what you decide and what things you'd like me to do.  
> I'll be working on the
> LiquidDirt until then.  

I don't know yet. Maybe I'll ask you to finish implementing the boot 
player command. When we have that, we can make a new release.