Attempt to fix #363

[buumi]

Attempt to fix #363. SetUnitSensorRadius("airLos" , ..) seems to also set los. Sounds like a engine bug?

[sanguinariojoe]

I'm afraid so... https://github.com/spring/spring/blob/develop/rts/Lua/LuaSyncedCtrl.cpp#L2492

Either if you call to set "los" or "airLos", both are set (note that the call to ChangeLos is always setting both at the same time):

		case hashString("los"): {
			unit->ChangeLos(unit->realLosRadius = radius, unit->realAirLosRadius);
			lua_pushnumber(L, unit->losRadius);
		} break;
		case hashString("airLos"): {
			unit->ChangeLos(unit->realAirLosRadius = radius, radius);
			lua_pushnumber(L, unit->airLosRadius);
		} break;

It seems like they did on forward purpose, but probably worths submitting an issue.

P.S. In previous version the game were ignoring defaultLos, after this PR it is ignoring defaultAirLos, right?

[buumi]

Actually writing 'los' is only setting 'los' based on my testing, while writing 'airLos' is setting both of them. Before the change:

[f=0000453] Setting los to: 75
[f=0000453] Setting airLos to: 480
[f=0000453] Setting radar to: 750
[f=0000453] Setting seismic to: 0
[f=0000453] airLos after writing is: 480
[f=0000453] los after writing is: 480
[f=0000453] radar after writing is: 750
[f=0000453] seismic after writing is: 0

After the change:

[f=0000549] Setting airLos to: 480
[f=0000549] Setting los to: 75
[f=0000549] Setting radar to: 750
[f=0000549] Setting seismic to: 0
[f=0000549] airLos after writing is: 480
[f=0000549] los after writing is: 75
[f=0000549] radar after writing is: 750
[f=0000549] seismic after writing is: 0

So unless there's something I missed, it should not ignore defaultAirLos.

[sanguinariojoe]

Oh yes! I'm idiot!

unit->ChangeLos(unit->realAirLosRadius = radius, radius); is setting both to radius, but unit->ChangeLos(unit->realLosRadius = radius, unit->realAirLosRadius); is setting just the first one to radius.

Go ahead, merge when you want!

Shall we submit an issue to the engine, or must we assume they want it like that?

[buumi]

No, you're not, you already got me thinking also :) This "call method and in parameters set object's instance variables" syntax is hurting my brains at the moment (if that's even what is happening there) but yes, I think you're correct.

It feels like a bug to me but can never know, I can submit ticket to engine Mantis at some point and devs can decide if it's wanted behaviour or not.