Modified to Change SensorWrapper to not implement Layer and fixed tests #5018
Conversation
| @@ -188,14 +188,15 @@ public IStatus execute(final IProgressMonitor monitor, final IAdaptable info) th | |||
|
|
|||
| protected List<EditableWrapper> itemsFor(final HasEditables parent) { | |||
| final List<EditableWrapper> res; | |||
There was a problem hiding this comment.
@ayeshare - have we lost the functionality where this code has been commented out?
There was a problem hiding this comment.
I have implemented equivalent code for speciaal handling of TacticalDataWrapper now. But I am not sure how I can test it.
| @@ -878,8 +878,6 @@ public static int mergeSensors(final Editable targetE, final Layers theLayers, f | |||
| for (int i = 0; i < subjects.length; i++) { | |||
| final SensorWrapper sensor = (SensorWrapper) subjects[i]; | |||
| if (sensor != target) { | |||
There was a problem hiding this comment.
@ayeshare - the next line has been deleted. Wasn't it doing something useful?
There was a problem hiding this comment.
No it wasnt doing anything useful. The methods append(target) was being called and it was not implemented for SensorWrapper. We removed implements Layer on TacticalWrapper, as none of the methods from layer was implemented on TacticalWrapper.
There was a problem hiding this comment.
This version of append is still in the file.
This call was essential to add the sensor cuts from the other sensor into the target sensor.
| _mySensors.setName(SENSORS_LAYER_NAME); | ||
|
|
||
| _myDynamicShapes = new SplittableLayer(true); | ||
| _myDynamicShapes.setName(DYNAMIC_SHAPES_LAYER_NAME); | ||
|
|
||
| _mySolutions = new BaseLayer(true); | ||
| _mySolutions = new RestrictedBaseLayer<>(true); |
There was a problem hiding this comment.
_mySolutions can be restricted to instances of TMAWrapper
| @@ -974,7 +975,6 @@ else if (point instanceof SensorWrapper) { | |||
| existing = oldS; | |||
|
|
|||
| // and append the data points | |||
There was a problem hiding this comment.
Are you sure the next line can be deleted?
There was a problem hiding this comment.
Yes, append was not implemented for TacticalDataWrapper and implements Layer was removed.
There was a problem hiding this comment.
Hmm. Here's the behaviour. When a new Sensor is being added, we check to see if there is already a sensor of that name. If there is a sensor of that name we add the points from the new sensor to the existing one.
This PR loses the last bit of functionality - we aren't adding the sensor contact wrappers from the new sensor to the old one. We're ignoring them.
…b.com/debrief/debrief into 5011_disallow_pasting_sensor_toplevel
|
@ayeshare - the failing unit tests highlight that the SensorWrapper logic has changed. I've tried to reinstate the correct logic, but a couple of unit tests are still failing. We've also got some Could you investigate the above please? |
…b.com/debrief/debrief into 5011_disallow_pasting_sensor_toplevel
|
Still some unit tests to fix, please @ayeshare https://travis-ci.com/github/debrief/debrief/builds/202717868#L5965 |
…b.com/debrief/debrief into 5011_disallow_pasting_sensor_toplevel
Fixes #5011