Buggy validation/processing in setMaterials/setShaders

[Capt. Murphy]

Looking at these posts and thinking about the way other methods that need an Object as a parameter work (e.g runScreen), I think the observed behaviour is buggy. There does seem to be some differences between runScreen and setShaders/setMaterials (setMaterialsInternal) in the way the parameters are processed and validated but I can't quite work out what might be going wrong.

[Eric Walch]

Looking at these posts and thinking about the way other methods that need an Object as a parameter work (e.g runScreen), I think the observed behaviour is buggy. There does seem to be some differences between runScreen and setShaders/setMaterials (setMaterialsInternal) in the way the parameters are processed and validated but I can't quite work out what might be going wrong.

Is it not the graphic card that is buffering stuff as I couldn't remember any buggy stuff with it and I used it in the console often enough when working on asteroid storm.

I just tested it with trunk:

Code: Select all

PS.target.setMaterials({"asteroid.png":{diffuse_map: {name:"astorm03.png", "repeat_s":true, "repeat_t":true},
                normal_map: {name:"astorm03_normal.png", "repeat_s":true, "repeat_t":true},
                specular_map: {name:"astorm_specular.png", "repeat_s":true, "repeat_t":true},
                shininess: 30}})

just sets correctly the new material on the asteroids. Similar code as Thargoid uses and this does work with me. So, its probably not just an JS issue.

[another_commander]

Yesterday I tried Thargoid's test code on a newly spawned cargopod and had the same symptoms as he reported.

Then, I run the same kind of code on a Dodo station and it worked fine. [wildGuess]Scanclass related maybe?[/wildGuess]

[Capt. Murphy]

Are you 100% sure it's setting correctly? Thargoid's and my experience is that it returns true, but with no visible change when the material/shader dictionary is used in full as the parameter. But when I set up the parameters as a variable and use the variable as the parameter for setMaterials, it returns true and there is an immediate visible change.

[another_commander]

I should specify that I used the setMaterials code, not the setShaders. The cargopod was returning true and nothing happened, while the Dodo immediately had visible changes on it (and returned true of course).

[Capt. Murphy]

Just a thought but should the texture name containing the dictionary always match the texture named in the .dat file?

Thargoid posted

T.setShaders({"griff_barrel_glowtest_diffuse.png": { textures: "griff_barrel_glowtest_diffuse.png", diffuse_map: "griff_barrel_glowtest_diffuse.png", fragment_shader: "oolite-default-shader.fragment", vertex_shader: "oolite-standard-vertex.vertex" }});

Maybe

T.setShaders({"griff_barrel_alt_diffuse.png": { textures: "griff_barrel_glowtest_diffuse.png", diffuse_map: "griff_barrel_glowtest_diffuse.png", fragment_shader: "oolite-default-shader.fragment", vertex_shader: "oolite-standard-vertex.vertex" }});

would work?

[Eric Walch]

Just a thought but should the texture name containing the dictionary always match the texture named in the .dat file?

What other texture name is there? The one in the .dat is the only reference the script has.

[another_commander]

One more test on setting materials for the cargopod:

0. Run Oolite with Debug Console active. No OXPs are necessary.
1. Spawn a cargopod: :spawn cargopod
2. Type T.setMaterials({"left_metal.png":{diffuse_color:"redColor"}}); in the console. The edges of the cargopod become red.
3. Type T.setMaterials({"barrel_metal.png":{diffuse_color:"greenColor"}}); On my system, the edges of the cargopod become normal again and its actual barrel_metal.png texture does not change.

Same model, one of its textures responds, the other does not. Smells bug to me.

[Capt. Murphy]

Code: Select all

materials =
		{
			"barrel_metal.png" =
			{
				shininess = 10;
				specular_color = ( 0.3, 0.3, 0.33, 1.0 );
			};
			"left_metal.png" =
			{
				shininess = 10;
				specular_color = ( 0.3, 0.3, 0.33, 1.0 );
			};
		};
		shaders =
		{
			"barrel_metal.png" =
			{
				specular_map = "oolite-barrel-specular.png";
				shininess = 60;
				specular_color = whiteColor;
			};
		};

I think that's because the shaders entry is overriding the materials entry of barrel_metal.png with shaders on. It goes green if you do it with shaders off (although the red is reset at the same time.... )

[another_commander]

Seems you are right. It does work with shaders off. So maybe it's just a false alarm. I think. Maybe.

[Capt. Murphy]

OK going back to what I originally thought was the bug try this with shaders off.

:spawn cargopod
T.setMaterials({"barrel_metal.png":{diffuse_map:"left_metal.png"}});

returns true and correctly applies the change.

However...

:spawn cargopod
x = "barrel_metal.png"
y = "left_metal.png"
T.setMaterials({x:{diffuse_map:y}})

returns true but doesn't change the texture.

However....

:spawn cargopod
x = "barrel_metal.png"
y = "left_metal.png"
z = new Object
z[x] = {diffuse_map:y}
T.setMaterials(z)

returns true and does change the texture.

I can't see why this should be, and even if there is an error in the 2nd construction it's a bug that it returns true.

Edit to add - by comparison for a different method that is also looking for an Object as a parameter both.

mission.runScreen({title:"test"})

and

x = "test"
mission.runScreen({title:x})

work just fine.

[Eric Walch]

:spawn cargopod
x = "barrel_metal.png"
y = "left_metal.png"
T.setMaterials({x:{diffuse_map:y}})

returns true but doesn't change the texture.

I think the problem here is that the property-name itself you use here is a variable. In all the other cases you give they are fixed strings.

It returns true when JS can create a mesh. Only after that it tried to apply the mesh on the ship. A failure of that is not reported back.

[Capt. Murphy]

Some more examples of bugs in this. This is changing the scale of the PaintColors on a Griff Cobra under full shaders via debug console.

Code: Select all

x={"griff_cobra_mk3_mainhull_diffuse_spec.png":{fragment_shader: "griff_cobra_mk3_player.fragment",textures:["griff_cobra_mk3_mainhull_diffuse_spec.png","griff_cobra_mk3_mainhull_normal.png","griff_cobra_mk3_mainhull_effects.png","griff_player_decal.png"],uniforms:{Decal1_Rotation:{type:"float",value:-0.73840},Decal1_Scale_and_Position:{type:"vector",value:"0.59 0.2 9.6"},PaintColor1:{scale: 0.85,type: "randomUnitVector"},PaintColor2:{scale: 0.85,type: "randomUnitVector"}, alertlevel: "alertCondition",damage_amount: "damage", engine_power: "speedFactor", hull_heat_level: "hullHeatLevel", uColorMap:{type:"texture",value:0},uDecalMap:{type:"texture",value:3},uEffectsMap:{type:"texture",value:2},uNormalMap:{type:"texture",value:1},uTime:"universalTime"},vertex_shader:"griff_normalmap_ships.vertex"}};

returns

Code: Select all

{
    "griff_cobra_mk3_mainhull_diffuse_spec.png": {
        fragment_shader: "griff_cobra_mk3_player.fragment",
        textures: ["griff_cobra_mk3_mainhull_diffuse_spec.png", "griff_cobra_mk3_mainhull_normal.png", "griff_cobra_mk3_mainhull_effects.png", "griff_player_decal.png"],
        uniforms: {
            Decal1_Rotation: {
                type: "float",
                value: -0.7384
            },
            Decal1_Scale_and_Position: {
                type: "vector",
                value: "0.59 0.2 9.6"
            },
            PaintColor1: {
                scale: 0.85,
                type: "randomUnitVector"
            },
            PaintColor2: {
                scale: 0.85,
                type: "randomUnitVector"
            },
            alertlevel: "alertCondition",
            damage_amount: "damage",
            engine_power: "speedFactor",
            hull_heat_level: "hullHeatLevel",
            uColorMap: {
                type: "texture",
                value: 0
            },
            uDecalMap: {
                type: "texture",
                value: 3
            },
            uEffectsMap: {
                type: "texture",
                value: 2
            },
            uNormalMap: {
                type: "texture",
                value: 1
            },
            uTime: "universalTime"
        },
        vertex_shader: "griff_normalmap_ships.vertex"
    }
}

Following that with...

Code: Select all

player.ship.setShaders(x)

returns true but does not apply the changes.

Edit - setShaders() is fixed in trunk with revision 5032.

However

Code: Select all

player.ship.setMaterials({},x)

returns true and does apply the changes.

[Capt. Murphy]

Just a thought but should the texture name containing the dictionary always match the texture named in the .dat file?

What other texture name is there? The one in the .dat is the only reference the script has.

Having been experimenting with setMaterials a lot this week, it appears that attempting to setMaterials on an entity with the incorrect initial texture reference simply has no effect (it still returns true though ). But I'm wondering if there is any underlying ill effect?

I'm thinking of an 1.76.1 OXP to apply some scripted variations to NPC Griff ships (including applying some randomUnitVector based tints when shaders aren't available and maybe in simple shader mode), using hard coded versions of the original shipdata material/shader dictionaries. But there is no way in 1.76.1 (or in trunk as yet) to reliably validate that that the entity in question is definitely a Griff ship, so the script could accidentally attempt to apply a Griff material/shader dictionary to a non Griff ship. Initial testing indicates this doesn't cause any difficulty, but does anyone know any different?

[Capt. Murphy]

Following that with...

Code: Select all

player.ship.setShaders(x)

returns true but does not apply the changes.

setShaders() is fixed in trunk with revision 5032.