aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorJustin Clark-Casey (justincc)2013-03-18 20:42:08 +0000
committerJustin Clark-Casey (justincc)2013-03-18 20:48:50 +0000
commita7a9a8a614549c7492e4954189e9f4df2473ca1e (patch)
tree3b8f51a0d2740c731fa7502cd03328de5a76d4b0
parentMerge branch 'master' of melanie@opensimulator.org:/var/git/opensim (diff)
downloadopensim-SC_OLD-a7a9a8a614549c7492e4954189e9f4df2473ca1e.zip
opensim-SC_OLD-a7a9a8a614549c7492e4954189e9f4df2473ca1e.tar.gz
opensim-SC_OLD-a7a9a8a614549c7492e4954189e9f4df2473ca1e.tar.bz2
opensim-SC_OLD-a7a9a8a614549c7492e4954189e9f4df2473ca1e.tar.xz
Fix recent regression where an item worn to an attachment point that was already occupied did not remove the previous attachment (current behaviour)
Regression was commit ccd6f4 (Tue Mar 5 23:47:36 2013) Added regression test for this case.
-rw-r--r--OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs178
-rw-r--r--OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs64
2 files changed, 153 insertions, 89 deletions
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index b6a7481..2092d6f 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -289,16 +289,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
289 if (!Enabled) 289 if (!Enabled)
290 return false; 290 return false;
291 291
292 if (AttachObjectInternal(sp, group, attachmentPt, silent, temp)) 292 return AttachObjectInternal(sp, group, attachmentPt, silent, temp, false);
293 {
294 m_scene.EventManager.TriggerOnAttach(group.LocalId, group.FromItemID, sp.UUID);
295 return true;
296 }
297
298 return false;
299 } 293 }
300 294
301 private bool AttachObjectInternal(IScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool silent, bool temp) 295 /// <summary>
296 /// Internal method which actually does all the work for attaching an object.
297 /// </summary>
298 /// <returns>The object attached.</returns>
299 /// <param name='sp'></param>
300 /// <param name='group'>The object to attach.</param>
301 /// <param name='attachmentPt'></param>
302 /// <param name='silent'></param>
303 /// <param name='temp'></param>
304 /// <param name='resumeScripts'>If true then scripts are resumed on the attached object.</param>
305 private bool AttachObjectInternal(
306 IScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool silent, bool temp, bool resumeScripts)
302 { 307 {
303// m_log.DebugFormat( 308// m_log.DebugFormat(
304// "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})", 309// "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})",
@@ -322,44 +327,44 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
322 return false; 327 return false;
323 } 328 }
324 329
330 Vector3 attachPos = group.AbsolutePosition;
331
332 // TODO: this short circuits multiple attachments functionality in LL viewer 2.1+ and should
333 // be removed when that functionality is implemented in opensim
334 attachmentPt &= 0x7f;
335
336 // If the attachment point isn't the same as the one previously used
337 // set it's offset position = 0 so that it appears on the attachment point
338 // and not in a weird location somewhere unknown.
339 if (attachmentPt != 0 && attachmentPt != group.AttachmentPoint)
340 {
341 attachPos = Vector3.Zero;
342 }
343
344 // AttachmentPt 0 means the client chose to 'wear' the attachment.
345 if (attachmentPt == 0)
346 {
347 // Check object for stored attachment point
348 attachmentPt = group.AttachmentPoint;
349 }
350
351 // if we still didn't find a suitable attachment point.......
352 if (attachmentPt == 0)
353 {
354 // Stick it on left hand with Zero Offset from the attachment point.
355 attachmentPt = (uint)AttachmentPoint.LeftHand;
356 attachPos = Vector3.Zero;
357 }
358
325 // Remove any previous attachments 359 // Remove any previous attachments
326 List<SceneObjectGroup> existingAttachments = sp.GetAttachments(attachmentPt); 360 List<SceneObjectGroup> existingAttachments = sp.GetAttachments(attachmentPt);
327 361
328 // At the moment we can only deal with a single attachment 362 // At the moment we can only deal with a single attachment
329 if (existingAttachments.Count != 0 && existingAttachments[0].FromItemID != UUID.Zero) 363 if (existingAttachments.Count != 0 && existingAttachments[0].FromItemID != UUID.Zero)
330 DetachSingleAttachmentToInv(sp, group); 364 DetachSingleAttachmentToInv(sp, existingAttachments[0]);
331 365
332 lock (sp.AttachmentsSyncLock) 366 lock (sp.AttachmentsSyncLock)
333 { 367 {
334 Vector3 attachPos = group.AbsolutePosition;
335
336 // TODO: this short circuits multiple attachments functionality in LL viewer 2.1+ and should
337 // be removed when that functionality is implemented in opensim
338 attachmentPt &= 0x7f;
339
340 // If the attachment point isn't the same as the one previously used
341 // set it's offset position = 0 so that it appears on the attachment point
342 // and not in a weird location somewhere unknown.
343 if (attachmentPt != 0 && attachmentPt != group.AttachmentPoint)
344 {
345 attachPos = Vector3.Zero;
346 }
347
348 // AttachmentPt 0 means the client chose to 'wear' the attachment.
349 if (attachmentPt == 0)
350 {
351 // Check object for stored attachment point
352 attachmentPt = group.AttachmentPoint;
353 }
354
355 // if we still didn't find a suitable attachment point.......
356 if (attachmentPt == 0)
357 {
358 // Stick it on left hand with Zero Offset from the attachment point.
359 attachmentPt = (uint)AttachmentPoint.LeftHand;
360 attachPos = Vector3.Zero;
361 }
362
363 group.AttachmentPoint = attachmentPt; 368 group.AttachmentPoint = attachmentPt;
364 group.AbsolutePosition = attachPos; 369 group.AbsolutePosition = attachPos;
365 370
@@ -367,6 +372,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
367 UpdateUserInventoryWithAttachment(sp, group, attachmentPt, temp); 372 UpdateUserInventoryWithAttachment(sp, group, attachmentPt, temp);
368 373
369 AttachToAgent(sp, group, attachmentPt, attachPos, silent); 374 AttachToAgent(sp, group, attachmentPt, attachPos, silent);
375
376 if (resumeScripts)
377 {
378 // Fire after attach, so we don't get messy perms dialogs
379 // 4 == AttachedRez
380 group.CreateScriptInstances(0, true, m_scene.DefaultScriptEngine, 4);
381 group.ResumeScripts();
382 }
383
384 // Do this last so that event listeners have access to all the effects of the attachment
385 m_scene.EventManager.TriggerOnAttach(group.LocalId, group.FromItemID, sp.UUID);
370 } 386 }
371 387
372 return true; 388 return true;
@@ -391,8 +407,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
391 return null; 407 return null;
392 408
393// m_log.DebugFormat( 409// m_log.DebugFormat(
394// "[ATTACHMENTS MODULE]: RezSingleAttachmentFromInventory to point {0} from item {1} for {2}", 410// "[ATTACHMENTS MODULE]: RezSingleAttachmentFromInventory to point {0} from item {1} for {2} in {3}",
395// (AttachmentPoint)AttachmentPt, itemID, sp.Name); 411// (AttachmentPoint)AttachmentPt, itemID, sp.Name, m_scene.Name);
396 412
397 // TODO: this short circuits multiple attachments functionality in LL viewer 2.1+ and should 413 // TODO: this short circuits multiple attachments functionality in LL viewer 2.1+ and should
398 // be removed when that functionality is implemented in opensim 414 // be removed when that functionality is implemented in opensim
@@ -525,6 +541,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
525 return; 541 return;
526 } 542 }
527 543
544// m_log.DebugFormat(
545// "[ATTACHMENTS MODULE]: Detaching object {0} {1} for {2} in {3}",
546// so.Name, so.LocalId, sp.Name, m_scene.Name);
547
528 // Scripts MUST be snapshotted before the object is 548 // Scripts MUST be snapshotted before the object is
529 // removed from the scene because doing otherwise will 549 // removed from the scene because doing otherwise will
530 // clobber the run flag 550 // clobber the run flag
@@ -846,60 +866,42 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
846 return null; 866 return null;
847 } 867 }
848 868
849 // Remove any previous attachments
850 List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
851
852 // At the moment we can only deal with a single attachment
853 if (attachments.Count != 0)
854 DetachSingleAttachmentToInv(sp, attachments[0]);
855
856 lock (sp.AttachmentsSyncLock)
857 {
858// m_log.DebugFormat( 869// m_log.DebugFormat(
859// "[ATTACHMENTS MODULE]: Rezzed single object {0} for attachment to {1} on point {2} in {3}", 870// "[ATTACHMENTS MODULE]: Rezzed single object {0} for attachment to {1} on point {2} in {3}",
860// objatt.Name, sp.Name, attachmentPt, m_scene.Name); 871// objatt.Name, sp.Name, attachmentPt, m_scene.Name);
861 872
862 // HasGroupChanged is being set from within RezObject. Ideally it would be set by the caller. 873 // HasGroupChanged is being set from within RezObject. Ideally it would be set by the caller.
863 objatt.HasGroupChanged = false; 874 objatt.HasGroupChanged = false;
864 bool tainted = false; 875 bool tainted = false;
865 if (attachmentPt != 0 && attachmentPt != objatt.AttachmentPoint) 876 if (attachmentPt != 0 && attachmentPt != objatt.AttachmentPoint)
866 tainted = true; 877 tainted = true;
867 878
868 // FIXME: Detect whether it's really likely for AttachObject to throw an exception in the normal 879 // FIXME: Detect whether it's really likely for AttachObject to throw an exception in the normal
869 // course of events. If not, then it's probably not worth trying to recover the situation 880 // course of events. If not, then it's probably not worth trying to recover the situation
870 // since this is more likely to trigger further exceptions and confuse later debugging. If 881 // since this is more likely to trigger further exceptions and confuse later debugging. If
871 // exceptions can be thrown in expected error conditions (not NREs) then make this consistent 882 // exceptions can be thrown in expected error conditions (not NREs) then make this consistent
872 // since other normal error conditions will simply return false instead. 883 // since other normal error conditions will simply return false instead.
873 // This will throw if the attachment fails 884 // This will throw if the attachment fails
874 try 885 try
875 { 886 {
876 AttachObjectInternal(sp, objatt, attachmentPt, false, false); 887 AttachObjectInternal(sp, objatt, attachmentPt, false, false, true);
877 } 888 }
878 catch (Exception e) 889 catch (Exception e)
879 { 890 {
880 m_log.ErrorFormat( 891 m_log.ErrorFormat(
881 "[ATTACHMENTS MODULE]: Failed to attach {0} {1} for {2}, exception {3}{4}", 892 "[ATTACHMENTS MODULE]: Failed to attach {0} {1} for {2}, exception {3}{4}",
882 objatt.Name, objatt.UUID, sp.Name, e.Message, e.StackTrace); 893 objatt.Name, objatt.UUID, sp.Name, e.Message, e.StackTrace);
883
884 // Make sure the object doesn't stick around and bail
885 sp.RemoveAttachment(objatt);
886 m_scene.DeleteSceneObject(objatt, false);
887 return null;
888 }
889
890 if (tainted)
891 objatt.HasGroupChanged = true;
892 894
893 // Fire after attach, so we don't get messy perms dialogs 895 // Make sure the object doesn't stick around and bail
894 // 4 == AttachedRez 896 sp.RemoveAttachment(objatt);
895 objatt.CreateScriptInstances(0, true, m_scene.DefaultScriptEngine, 4); 897 m_scene.DeleteSceneObject(objatt, false);
896 objatt.ResumeScripts(); 898 return null;
899 }
897 900
898 // Do this last so that event listeners have access to all the effects of the attachment 901 if (tainted)
899 m_scene.EventManager.TriggerOnAttach(objatt.LocalId, itemID, sp.UUID); 902 objatt.HasGroupChanged = true;
900 903
901 return objatt; 904 return objatt;
902 }
903 } 905 }
904 906
905 /// <summary> 907 /// <summary>
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 0ee01c7..624adcf 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -293,7 +293,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
293 // Check appearance status 293 // Check appearance status
294 Assert.That(sp.Appearance.GetAttachments().Count, Is.EqualTo(1)); 294 Assert.That(sp.Appearance.GetAttachments().Count, Is.EqualTo(1));
295 Assert.That(sp.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo((int)AttachmentPoint.Chest)); 295 Assert.That(sp.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo((int)AttachmentPoint.Chest));
296
297 Assert.That(scene.GetSceneObjectGroups().Count, Is.EqualTo(1)); 296 Assert.That(scene.GetSceneObjectGroups().Count, Is.EqualTo(1));
298 297
299 // Check events 298 // Check events
@@ -301,6 +300,69 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
301 } 300 }
302 301
303 /// <summary> 302 /// <summary>
303 /// Test wearing an attachment from inventory, as opposed to explicit choosing the rez point
304 /// </summary>
305 [Test]
306 public void TestWearAttachmentFromInventory()
307 {
308 TestHelpers.InMethod();
309// TestHelpers.EnableLogging();
310
311 Scene scene = CreateTestScene();
312 UserAccount ua1 = UserAccountHelpers.CreateUserWithInventory(scene, 0x1);
313 ScenePresence sp = SceneHelpers.AddScenePresence(scene, ua1.PrincipalID);
314
315 InventoryItemBase attItem1 = CreateAttachmentItem(scene, ua1.PrincipalID, "att1", 0x10, 0x20);
316 InventoryItemBase attItem2 = CreateAttachmentItem(scene, ua1.PrincipalID, "att2", 0x11, 0x21);
317
318 {
319 scene.AttachmentsModule.RezSingleAttachmentFromInventory(sp, attItem1.ID, (uint)AttachmentPoint.Default);
320
321 // default attachment point is currently the left hand.
322 Assert.That(sp.HasAttachments(), Is.True);
323 List<SceneObjectGroup> attachments = sp.GetAttachments();
324 Assert.That(attachments.Count, Is.EqualTo(1));
325 SceneObjectGroup attSo = attachments[0];
326 Assert.That(attSo.Name, Is.EqualTo(attItem1.Name));
327 Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.LeftHand));
328 Assert.That(attSo.IsAttachment);
329
330 // Check appearance status
331 Assert.That(sp.Appearance.GetAttachments().Count, Is.EqualTo(1));
332 Assert.That(sp.Appearance.GetAttachpoint(attItem1.ID), Is.EqualTo((int)AttachmentPoint.LeftHand));
333 Assert.That(scene.GetSceneObjectGroups().Count, Is.EqualTo(1));
334
335 // Check events
336 Assert.That(m_numberOfAttachEventsFired, Is.EqualTo(1));
337 }
338
339 // Test wearing a second attachment at the same position
340 // Until multiple attachments at one point is implemented, this will remove the first attachment
341 // This test relies on both attachments having the same default attachment point (in this case LeftHand
342 // since none other has been set).
343 {
344 scene.AttachmentsModule.RezSingleAttachmentFromInventory(sp, attItem2.ID, (uint)AttachmentPoint.Default);
345
346 // default attachment point is currently the left hand.
347 Assert.That(sp.HasAttachments(), Is.True);
348 List<SceneObjectGroup> attachments = sp.GetAttachments();
349 Assert.That(attachments.Count, Is.EqualTo(1));
350 SceneObjectGroup attSo = attachments[0];
351 Assert.That(attSo.Name, Is.EqualTo(attItem2.Name));
352 Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.LeftHand));
353 Assert.That(attSo.IsAttachment);
354
355 // Check appearance status
356 Assert.That(sp.Appearance.GetAttachments().Count, Is.EqualTo(1));
357 Assert.That(sp.Appearance.GetAttachpoint(attItem2.ID), Is.EqualTo((int)AttachmentPoint.LeftHand));
358 Assert.That(scene.GetSceneObjectGroups().Count, Is.EqualTo(1));
359
360 // Check events
361 Assert.That(m_numberOfAttachEventsFired, Is.EqualTo(3));
362 }
363 }
364
365 /// <summary>
304 /// Test specific conditions associated with rezzing a scripted attachment from inventory. 366 /// Test specific conditions associated with rezzing a scripted attachment from inventory.
305 /// </summary> 367 /// </summary>
306 [Test] 368 [Test]