Unit equals implementation is "bad"

[Bytekeeper]

I guess the code is copied from BWMirror, which only compares the ids to determine equality.
This is rather bad for BWAPI4J where a type change also results in a new Unit object with the same id.

If, for example used as a Map key the following code or similar might be used:

for (Entry<Unit,Something> u: Units.entrySet()) {
...u.getKey().getPosition().getDistance(u.getValue().goal)...

Now, getKey will always return the initial unit, for example a larva... but now it's a zergling! The position of the larva will never be updated and thus this usage is in vain.

I think the equals method should include the unit type too at the very least. Opinions?

[Jabbo16]

I agree, I discovered some problems recently because my maps keys (Unit) are not updated when the units morph.

[JasperGeurtz]

Tanks however stay the same class even if their unittype changes (siege/unsiege), so just got to be careful with that.

[Bytekeeper]

@JasperGeurtz True, but I think that's "safe". The main problem here is the types that actually change classes. Some code could hold references and not detect that the instance is invalid.