Revision 4905 bug?

For test results, bug reports, announcements of new builds etc.

Moderators: winston, another_commander, Getafix

User avatar
Capt. Murphy
Commodore
Commodore
Posts: 1127
Joined: Fri Feb 25, 2011 8:46 am
Location: UK South Coast.

Revision 4905 bug?

Post by Capt. Murphy »

Hi,

Equipment listed in a shipyard entry as standard equipment that has "available_to_all" set to false is not being fitted on purchasing the ship since this revision. Looks to my untrained eye that the new code checks for optional equipment but not standard. I think this revision also made it into the maintenance branch.

Code: Select all

if (![eqType isAvailableToAll])  
                {
                        // find options that agree with this ship. Only player ships have these options.
                        OOShipRegistry          *registry = [OOShipRegistry sharedRegistry];
                        NSDictionary            *shipyardInfo = [registry shipyardInfoForKey:[self shipDataKey]];
                        NSMutableSet            *options = [NSMutableSet setWithArray:[shipyardInfo oo_arrayForKey:KEY_OPTIONAL_EQUIPMENT]];
                        if (![options containsObject:equipmentKey])  return NO;
                }
[EliteWiki] Capt. Murphy's OXPs
External JavaScript resources - W3Schools & Mozilla Developer Network
Win 7 64bit, Intel Core i5 with HD3000 (driver rev. 8.15.10.2696 - March 2012), Oolite 1.76.1
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: Revision 4905 bug?

Post by Eric Walch »

Capt. Murphy wrote:
Equipment listed in a shipyard entry as standard equipment that has "available_to_all" set to false is not being fitted on purchasing the ship since this revision. Looks to my untrained eye that the new code checks for optional equipment but not standard. I think this revision also made it into the maintenance branch.
That is correct. A player ship can only be awarded by script with equipment that is "available_to_all" or is listed as option. Standard equipment, the ship already starts with. I didn't see a reason to look there also.

EDIT: I now see the problem you mention. Already on buying some of the extra equipment is now also skipped. From the internal ships I see it now on the ferdi.
Last edited by Eric Walch on Sat May 19, 2012 6:32 pm, edited 1 time in total.
User avatar
Capt. Murphy
Commodore
Commodore
Posts: 1127
Joined: Fri Feb 25, 2011 8:46 am
Location: UK South Coast.

Re: Revision 4905 bug?

Post by Capt. Murphy »

Eric Walch wrote:
Capt. Murphy wrote:
Standard equipment, the ship already starts with. I didn't see a reason to look there also.
That's the problem Eric, ships aren't starting with what should be standard if available_to_all is false. Thargoids Vortex has a good example.
[EliteWiki] Capt. Murphy's OXPs
External JavaScript resources - W3Schools & Mozilla Developer Network
Win 7 64bit, Intel Core i5 with HD3000 (driver rev. 8.15.10.2696 - March 2012), Oolite 1.76.1
another_commander
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 6634
Joined: Wed Feb 28, 2007 7:54 am

Re: Revision 4905 bug?

Post by another_commander »

I don't have too much time to do a proper test right now, but I guess adding this line:

Code: Select all

[options addObjectsFromArray:[[shipyardInfo oo_dictionaryForKey:KEY_STANDARD_EQUIPMENT] oo_arrayForKey:KEY_EQUIPMENT_EXTRAS]];
before the inner if line should probably fix it. I think the optional equipment list should include by default everything that is standard as well.
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: Revision 4905 bug?

Post by Eric Walch »

another_commander wrote:
I don't have too much time to do a proper test right now, .....
before the inner if line should probably fix it. I think the optional equipment list should include by default everything that is standard as well.
I just tested it and that should be enough. The bug was noticeable with the fer-de-lance that has quite a lot extra equipment defined that is not available_to_all. e.g. the fuel injection was missing. With your fix the fer-de-lance can have all extra equipment again as it should.

Just committed to maintenance. Will do trunk tomorrow (I don't know how to commit simultaneously into two branches with XCode)
User avatar
CommonSenseOTB
---- E L I T E ----
---- E L I T E ----
Posts: 1397
Joined: Wed May 04, 2011 10:42 am
Location: Saskatchewan, Canada

Re: Revision 4905 bug?

Post by CommonSenseOTB »

I've come across something that may be a related problem and I'm not sure if it is working in the way it was intended. I'm aware of the above change to the shipyard.plist for standard equipment so one part of my observation has been addressed.

The other part involves the shipdata.plist when weapons are defined, for example ' aft_weapon_type = "EQ_WEAPON_MILITARY_LASER"; '. When this is in the shipdata listing for the player ship there are 2 possibilities. If the shipyard listing has a facing listed for the weapon defined in shipdata, then the weapon can be changed 'player.ship.aftWeapon = null' but if there is no facing defined for that weapon, in this case say that weapon facing is 1, then the result will be that the aft weapon cannot be removed, it is stuck and cannot be changed. Is this intended behaviour?

The concern is that many player-ship shipdata listings are like_shipped from the NPC listing and as a result, if the facings aren't defined in the player shipyard listing then the ship will come with unchangeable weapons.

I can work around this for the custom lasers oxp, as those unmoveable weapons will just stay put, but for the player ships that operate in this sloppy manner there will be problems with any oxp wanting to remove or change the weapon if there is no weapon facing in the shipyard.plist file. Part of my oxp will have simulated damage effects for the weapons and would like that to be applied to all weapons on a ship.
Take an idea from one person and twist or modify it in a different way as a return suggestion so another person can see a part of it that can apply to the oxp they are working on.


CommonSense 'Outside-the-Box' Design Studios Ltd.
WIKI+OXPs
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: Revision 4905 bug?

Post by Eric Walch »

CommonSenseOTB wrote:
The other part involves the shipdata.plist when weapons are defined, for example ' aft_weapon_type = "EQ_WEAPON_MILITARY_LASER"; '. When this is in the shipdata listing for the player ship there are 2 possibilities. If the shipyard listing has a facing listed for the weapon defined in shipdata, then the weapon can be changed 'player.ship.aftWeapon = null' but if there is no facing defined for that weapon, in this case say that weapon facing is 1, then the result will be that the aft weapon cannot be removed, it is stuck and cannot be changed. Is this intended behaviour?

The concern is that many player-ship shipdata listings are like_shipped from the NPC listing and as a result, if the facings aren't defined in the player shipyard listing then the ship will come with unchangeable weapons.
There is no relation with above bug. And I am not sure if it is a bug. If a facing was defined to have no weapon there should be no option to add a weapon at that location. If a ship was defined with a weapon at such a location, I would say it is a feature that the weapon is still there but unchangeable.

From the code I can't see why not having any facings defined should give this behaviour, as facings defaults to 'all_facings' when it was not defined. You could verify the facings as seen by Oolite in the console with "PS.weaponFacings". I think weaponFacings a trunk-only property recently added by cim. Than you can double check the facings as seen by Oolite.
User avatar
Commander McLane
---- E L I T E ----
---- E L I T E ----
Posts: 9520
Joined: Thu Dec 14, 2006 9:08 am
Location: a Hacker Outpost in a moderately remote area
Contact:

Re: Revision 4905 bug?

Post by Commander McLane »

CommonSenseOTB wrote:
The concern is that many player-ship shipdata listings are like_shipped from the NPC listing and as a result, if the facings aren't defined in the player shipyard listing then the ship will come with unchangeable weapons.
But that would be an OXP bug, not an Oolite bug. Every OXP author is responsible for getting their shipdata.plist and shipyard.plist right. It's all well-documented, and it's not exactly rocket science. :)
User avatar
CommonSenseOTB
---- E L I T E ----
---- E L I T E ----
Posts: 1397
Joined: Wed May 04, 2011 10:42 am
Location: Saskatchewan, Canada

Re: Revision 4905 bug?

Post by CommonSenseOTB »

Ok Eric Walch and Commander McLane, I'll accept that this is a feature and it is up to oxp shipbuilders to have as many facings defined in the shipyard.plist as there are weapons defined in the shipdata.plist for the player's ship. What I was referring to Eric Walch was the weapon facings being 1(forward weapon only) so the player can buy lasers on the front but the shipdata having an aft laser so it can't be changed, not just by purchase but by player.ship.aftWeapon = null. And yes I agree that it is lousy ship design and not rocket science that causes this inconsistancy but these ships are scattered all over in different oxps and any oxp that wants to remove that aft weapon in this example will not be able to. Doesn't bother me, I will work around it and make sure virtual lasers aren't offered for positions where the laser can't be removed. This post will be a reminder for oxpers that want to change or remove a laser from the player ship that they also need to double check first if it has indeed been removed before proceeding with thier code.

Thanks for the explanation gentlemen. :D
Take an idea from one person and twist or modify it in a different way as a return suggestion so another person can see a part of it that can apply to the oxp they are working on.


CommonSense 'Outside-the-Box' Design Studios Ltd.
WIKI+OXPs
User avatar
Capt. Murphy
Commodore
Commodore
Posts: 1127
Joined: Fri Feb 25, 2011 8:46 am
Location: UK South Coast.

Re: Revision 4905 bug?

Post by Capt. Murphy »

Eric Walch wrote:
There is no relation with above bug. And I am not sure if it is a bug. If a facing was defined to have no weapon there should be no option to add a weapon at that location. If a ship was defined with a weapon at such a location, I would say it is a feature that the weapon is still there but unchangeable.
Although an OXP bug, perhaps it would be better if the core game code prevented the fitting of a shipdata.plist defined weapon if the shipyard.plist specifically defines that the facing shouldn't be available.

The other way round doesn't seem so intuitive i.e override and change the shipyard.plist defined facings if the shipdata.plist defines a weapon at what should be a non-available facing.

Leaving it as it is strikes me if nothing else as untidy...

CSOTB - Can you give an example of an ship OXP with the problem?
[EliteWiki] Capt. Murphy's OXPs
External JavaScript resources - W3Schools & Mozilla Developer Network
Win 7 64bit, Intel Core i5 with HD3000 (driver rev. 8.15.10.2696 - March 2012), Oolite 1.76.1
User avatar
Smivs
Retired Assassin
Retired Assassin
Posts: 8408
Joined: Tue Feb 09, 2010 11:31 am
Location: Lost in space
Contact:

Re: Revision 4905 bug?

Post by Smivs »

Capt. Murphy wrote:
Leaving it as it is strikes me if nothing else as untidy...
Agreed.
My 2Cr-worth:- If a player ship does not have the facing/s specified in shipyard.plist, any weapon placed in that position via shipdata should be ignored...it should not work or even exist, and therefore cannot be replaceable.
Commander Smivs, the friendliest Gourd this side of Riedquat.
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: Revision 4905 bug?

Post by Eric Walch »

Smivs wrote:
My 2Cr-worth:- If a player ship does not have the facing/s specified in shipyard.plist, any weapon placed in that position via shipdata should be ignored....
You are right, but it needs several of additional checks throughout the whole code. At least on buying and probably also on loading a saved game. Probably not worth the trouble as it only happens with wrong designed ships (oxp errors).

I see another problem with npc ships. Currently weapon facing was a player only property. There was no need for this key with npc ships. The facings were determined in the shipData.plist. Now lasers become buyable, it seams silly that a player adder can only have a front weapon, while a npc adder can have four.
Probably we need a shipdata key for it.
User avatar
Smivs
Retired Assassin
Retired Assassin
Posts: 8408
Joined: Tue Feb 09, 2010 11:31 am
Location: Lost in space
Contact:

Re: Revision 4905 bug?

Post by Smivs »

Eric Walch wrote:
Smivs wrote:
My 2Cr-worth:- If a player ship does not have the facing/s specified in shipyard.plist, any weapon placed in that position via shipdata should be ignored....
You are right, but it needs several of additional checks throughout the whole code. At least on buying and probably also on loading a saved game. Probably not worth the trouble as it only happens with wrong designed ships (oxp errors).
Yes, I realise that. It was more an aspiration than a suggestion, and as you say, a lot of effort for no obvious benefit.
Commander Smivs, the friendliest Gourd this side of Riedquat.
User avatar
cim
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 4072
Joined: Fri Nov 11, 2011 6:19 pm

Re: Revision 4905 bug?

Post by cim »

Oddities with assignment of player lasers, and a weapon_facings property for NPC ships, added in r4957.
User avatar
Capt. Murphy
Commodore
Commodore
Posts: 1127
Joined: Fri Feb 25, 2011 8:46 am
Location: UK South Coast.

Re: Revision 4905 bug?

Post by Capt. Murphy »

cim wrote:
Oddities with assignment of player lasers, and a weapon_facings property for NPC ships, added in r4957.
Looking at the SVN log and the commit I'm wondering if that was meant to read Oddities with assignment of player lasers fixed, and a weapon_facings property for NPC ships added, in r4957?
[EliteWiki] Capt. Murphy's OXPs
External JavaScript resources - W3Schools & Mozilla Developer Network
Win 7 64bit, Intel Core i5 with HD3000 (driver rev. 8.15.10.2696 - March 2012), Oolite 1.76.1
Post Reply