Correction of Harm detection table copy

[MacFlorent]

Harm identification is currently only done for the first radar that detects the missile. Later detections by other radars will not try to identify again.

This is because when Skynet evaluates the new radars that should try for identification of the contact, it will compare (in function getNewRadarsThatHaveDetectedContact) :

• The list [1] of radars currently seeing the contact (contact:getAbstractRadarElementsDetected)
• The list [2] of radars having already tried to identify it as HARM (self.contactRadarsEvaluated[contact])
  And try the identification only for the ones that are in [1] but not in [2].
  Then, it will point list [2] to list [1] (to not retry the evaluation for these radars later).

But then, since list [2] points list [1] , each time a radar is added to list [1], it will in effect also be in list [2]. And from now on, there will never be any difference between [1] and [2], and so never any radar with which to try the HARM identification.

Proposed correction is to copy the content of [1] to [2] instead of just pointing to it.

As a sidenote, it seems that the augmented probability when multiple radars are tracking the contact is almost never triggered because it would need the multiple radars to pick up the contact in the same Skynet loop, and this seldom happens in my experience.

[walder]

Good Catch, seems I tripped over OO basics....

I tested this functionality with this unit test:

The mock data in the test creates a new array for each check, therefore the test passes.....

function TestSkynetIADSHARMDetection:testGetNewRadarsThatHaveDetectedContact()
	local mockContact = {}
	local mockRadar1 = {"MockRadar1"}
	local mockRadar2 = {"MockRadar2"}
	function mockContact:getAbstractRadarElementsDetected()
		return {mockRadar1, mockRadar2}
	end
	local result = self.harmDetection:getNewRadarsThatHaveDetectedContact(mockContact)
	lu.assertEquals(result, {mockRadar1, mockRadar2})

	local result2 = self.harmDetection:getNewRadarsThatHaveDetectedContact(mockContact)
	lu.assertEquals(result2, {})
	
	local mockRadar3 = {"MockRadar3"}
	function mockContact:getAbstractRadarElementsDetected()
		return {mockRadar1, mockRadar2, mockRadar3}
	end
	local result3 = self.harmDetection:getNewRadarsThatHaveDetectedContact(mockContact)
	lu.assertEquals(result3, {mockRadar3})	
	
	local mockRadar4 = {"MockRadar4"}
	function mockContact:getAbstractRadarElementsDetected()
		return {mockRadar4, mockRadar1, mockRadar2, mockRadar3}
	end
	local result4 = self.harmDetection:getNewRadarsThatHaveDetectedContact(mockContact)
	lu.assertEquals(result4, {mockRadar4})	
end

In regard to not picking up in the same loop:
A lot of elaborate code for not much effect. What would you propose? Shall a negatively detected HARM be reevaluated in the next loop if a new radar has detected the HARM? This would increase detection chance, on the other hand HARMs are not in the air for very long.

Another Idea could be to get rid of the NOT_HARM state and just leave the contact as unknown, if the harm detection of a radar failes.

[MacFlorent]

Regarding the second point I only mentioned it as a sidenote because it's what led me down this rabbit hole : I was trying and failing to see Skynet try the 80% identification probability. But this one is not a clear coding anomaly as was the first point, so I don't feel very competent to propose an adequate improvement.

Intuitively I feel your first idea is great.
"Shall a negatively detected HARM be reevaluated in the next loop if a new radar has detected the HARM?"
If you keep my changes, this is what will happen already, but each time with only the detection probability of the new radar. What you could do is when you compute the percentage, take into account all the radars that are tracking and not only the new ones. As you said this will noticeably up the chances of identification, and I don't know if it is realistic.

[walder]

I had the same thought, take into account all radars that have detected the HARM in the next loop, event the ones that previously failed and calculate a combined probability for a new detection chance. This would only happen if a new radar picks up a HARM. Anyway best this would have to be tested in a few real life game scenarios, to test playability. Thanks for you thoughts on this topic!

[walder]

Fixed the code and updated unit test, what still needs to be done is:
I had the same thought, take into account all radars that have detected the HARM in the next loop, event the ones that previously failed and calculate a combined probability for a new detection chance. This would only happen if a new radar picks up a HARM. Anyway best this would have to be tested in a few real life game scenarios, to test playability. Thanks for you thoughts on this topic!

[MacFlorent]

This has been addressed in [3.2.0] (https://github.com/walder/Skynet-IADS/releases/tag/3.2.0), and implemented a bit differently that what we were using for a year in our branch.