Different color for non hostiles with bounty?

[Screet]

Hmm, I don't know why I didn't notice 'hasEquipmentItem item' before.

Well, have a look at the code:

Code: Select all

- (BOOL) hasScoop
{
	return [self hasEquipmentItem:@"EQ_FUEL_SCOOPS"];
}


- (BOOL) hasECM
{
	return [self hasEquipmentItem:@"EQ_ECM"];
}


- (BOOL) hasCloakingDevice
{
	return [self hasEquipmentItem:@"EQ_CLOAKING_DEVICE"];
}


- (BOOL) hasMilitaryScannerFilter
{
#if USEMASC
	return [self hasEquipmentItem:@"EQ_MILITARY_SCANNER_FILTER"];
#else
	return NO;
#endif
}


- (BOOL) hasMilitaryJammer
{
#if USEMASC
	return [self hasEquipmentItem:@"EQ_MILITARY_JAMMER"];
#else
	return NO;
#endif
}


- (BOOL) hasExpandedCargoBay
{
	return [self hasEquipmentItem:@"EQ_CARGO_BAY"];
}


- (BOOL) hasShieldBooster
{
	return [self hasEquipmentItem:@"EQ_SHIELD_BOOSTER"];
}


- (BOOL) hasMilitaryShieldEnhancer
{
	return [self hasEquipmentItem:@"EQ_NAVAL_SHIELD_BOOSTER"];
}


- (BOOL) hasHeatShield
{
	return [self hasEquipmentItem:@"EQ_HEAT_SHIELD"];
}


- (BOOL) hasFuelInjection
{
	return [self hasEquipmentItem:@"EQ_FUEL_INJECTION"];
}


- (BOOL) hasEnergyBomb
{
	return [self hasEquipmentItem:@"EQ_ENERGY_BOMB"];
}


- (BOOL) hasEscapePod
{
	return [self hasEquipmentItem:@"EQ_ESCAPE_POD"];
}


- (BOOL) hasDockingComputer
{
	return [self hasEquipmentItem:@"EQ_DOCK_COMP"];
}


- (BOOL) hasGalacticHyperdrive
{
	return [self hasEquipmentItem:@"EQ_GAL_DRIVE"];
}


- (float) shieldBoostFactor
{
	float boostFactor = 1.0f;
	if ([self hasShieldBooster])  boostFactor += 1.0f;
	if ([self hasMilitaryShieldEnhancer])  boostFactor += 1.0f;
	
	return boostFactor;
}


- (float) maxForwardShieldLevel
{
	return BASELINE_SHIELD_LEVEL * [self shieldBoostFactor];
}


- (float) maxAftShieldLevel
{
	return BASELINE_SHIELD_LEVEL * [self shieldBoostFactor];
}


- (float) shieldRechargeRate
{
	return [self hasMilitaryShieldEnhancer] ? 3.0f : 2.0f;
}

Note that these are methods, some of which are called for every ship, every time their course needs to be adjusted. They may return a boolean, but they are methods which call hasEquipmentItem, some of these calls multiple times.

Furthermore, while my code addition only calls this in the default for determination of scanner color, Oolite itself does something very similar at the beginning:

Code: Select all

- (BOOL) isJammingScanning
{
	return ([self hasMilitaryJammer] && military_jammer_active);
}

That's why I say that what I did provide is exactly written in the style Oolite does these things. Thus, if you complain, let's talk about how Oolite requires to be fixed and whether the current break with object orientation for equipment is really a good idea to keep.

Screet

[JensAyton]

you really shouldn't need to call hasEquimpentItem item every time you redraw the screen. Querying an internal boolean is much much faster.

If that is the reason why the proposed change was called a hack, then please admit that Oolite simply has been hacked together.

Oolite is very definitely hacked together. A lot of improvements have been made, and continue to be made, but it will never be a paragon of clean code.

Having said that, I disagree with Kaks in this instance: a single method call per frame is not a performance issue worth worrying about, and one type of cleanup I’ve done a lot of is replacing boolean flags and other direct variable accesses with method calls. In particular, having a flag in addition to an equipment item would be premature optimization introducing an entirely unnecessary possibility of inconsistent state (although this doesn’t apply in the case of having only the flag and no magic equipment item).

[Screet]

In particular, having a flag in addition to an equipment item would be premature optimization introducing an entirely unnecessary possibility of inconsistent state (although this doesn’t apply in the case of having only the flag and no magic equipment item).

Thinking C++ here:
Maybe the code should really be updated to see equipment as objects and then have a pointer to those regularly checked items instead of flag variables and such. The equipment would have a link to it's owner and if it's destroyed, tells the owner to set that variable to NULL.

In that case, the equipment itself could even have a status flag (for damaged) and handle it's own status for activation (and thus properly deactivate upon damage) and specific code to perform instead of requiring the ship to do it for the equipment.

It would be some rather big change though and would have to be well thought through before going that path

Screet

[JensAyton]

Thinking C++ here:
Maybe the code should really be updated to see equipment as objects and then have a pointer to those regularly checked items instead of flag variables and such. The equipment would have a link to it's owner and if it's destroyed, tells the owner to set that variable to NULL.

I’d like equipment items to be scripted objects that modify ship state when they’re added/removed/disabled. It’s one of, oh, about 73½ major design changes that would be nice to have, but I’m not even going to consider working on at this point for reasons which, I hope, are well-known. :-)

[Kaks]

Oolite could do well with some refactoring

Sure.
If you look at the commits made in the past year, you might notice a very high percentage of refactoring changes. Inefficiencies do get in from time to time in projects as big as this one, usually as a byproduct of trying to get something 'out there' right now, as opposed to a few weeks from now.

And inefficiencies tend to stay in as a byproduct of trying to get some major enhancements 'out there' within a reasonable time, instead of an unreasonable amount of time. And yes, I've had my fair share of premature optimisation in the past, though hopefully I'm getting better...

--

Only 73½ major changes? Excellent! Only 4 years to go then!
On a serious note, and possibly prematurely, I think we might start considering having a feature freeze soon.
We seem to already have a fairly solid base for release - features and bug-fixes wise - once the new mission screen api & hopefully new planet rendering code are in place. (Probably )

Of course, we can always do with more bug fixes!

My .2 credits...

[Commander McLane]

... I think we might start considering having a feature freeze soon.

But only after working in all my recent scripting requests, please?