377 [Bug] Lock on military tent goes invalid

[Alectrys 2,040]

Until this issue is fixed (it happens with other locked items as well), if you remember - whenever crossing servers with a tent, unlock them until you reach your destination and then re-lock them when you want to use it.  Then remember to do the same on your return trip.

 

Thanks for your details, Nomadikhan.

[Aeryck 862]

5 hours ago, Alectrys said:

Until this issue is fixed (it happens with other locked items as well), if you remember - whenever crossing servers with a tent, unlock them until you reach your destination and then re-lock them when you want to use it.  Then remember to do the same on your return trip.

 

Thanks for your details, Nomadikhan.

 

Tried this, but the lock often disappears, or a new lock appears in your inventory. I cant say that I have documented this procedure or tested it fully, but I had this same idea and know it still didnt work 100% of the time.

[Benie 49]

How odd. I've never had this happen to me.. well, apart from exploring Cele for the first time (a few years ago), and being warned about the same message. But I was told it was a bug with the lock that comes with the tent (and not of this), and got a replacement for free.
I haven't had a problem since. I'm not saying I'll probably never have this happen (thankful it hasn't), I just haven't seen it. And now it's probably going to happen to me, since I've mentioned it. ?

Edited July 4, 2018 by Benie

[Alectrys 2,040]

Still reported, still on the list: https://forum.wurmonline.com/index.php?/topic/169587-duplicate-small-locked-chestserver-crossing/

[Alectrys 2,040]

This happens with chests and tents as well.  Reported again, add to the bug again.  https://forum.wurmonline.com/index.php?/topic/170439-locked-chests-and-the-disappearing-items/

[Sklo:D 2,210]

On 6/13/2019 at 5:31 PM, Alectrys said:

This happens with chests and tents as well.  Reported again, add to the bug again.  https://forum.wurmonline.com/index.php?/topic/170439-locked-chests-and-the-disappearing-items/

 

Any idea what is delaying the bug fix?

 

I looked through the code the last 30 minutes and found a possible edge case which could cause this issue. It is just a guess, but if it is really that causing the issue it could be fixed in a few lines of code once and for all hopefully. Quite interested in a fix as we have the problem quite frequently.

Edited June 14, 2019 by Sklo:D

[Sklo:D 2,210]

In the end my solution to fix this bug is a bit longer than 2 lines but it is still simple enough and should fix the problems. I couldn't test this, so it is a guess, but I don't see any other possibility how the bug could be caused. Maybe something to check for @Keenanor @Budda

 

 

So I would try to fix this bug like that:

 

PlayerTransfer.class

public static final void sendItem(final Item item, final DataOutputStream dos, final boolean dragged) throws UnsupportedEncodingException, IOException {

//.....

        dos.writeBoolean(item.getLocked());
        dos.writeLong(item.getLockId());
        if (item.getLockId() != -10L) {
            if (item.isHollow()) {

                Item lock;
                try {
                    lock = Items.getItem(item.getLockId());
                    dos.writeBoolean(true);
                    sendItem(lock, dos, dragged);
                }
                catch (NoSuchItemException ex) {
                          try {

                              //Lock not loaded yet, loading...
                             lock = new DbItem(itemid);
                          }
                         catch (NoSuchItemException nsi) {
                              try {

                                    //Lock frozen, loading....
                                   lock = new DbItem(itemid, true);
                             }
                             catch (NoSuchItemException nsi2) {
                                  PlayerTransfer.logger.log(Level.WARNING, "Lock with the id "+lock.getId()+" of item "+item.getId()+" not found.");
                             }
                     }              
                }

                if(lock != null) {

                   Items.putItem(lock); //optional since the item will leave the server 

                   dos.writeBoolean(true);
                   sendItem(lock, dos, dragged);

                }

                else {

                   dos.writeBoolean(false);

                }
            }
            else {
                dos.writeBoolean(false);

                PlayerTransfer.logger.log(Level.WARNING, "Item "+item.getId()+" not hollow but locked.");
            }
        }

//....

 

Also improving Items.getItem(...) so it is actually safe and checking every possibility could be something which wouldn't be bad.

Edited June 15, 2019 by Sklo:D

[Brash_Endeavors 4,735]

this was first posted three years ago  :   Posted June 6, 2016

 

>"my solution to fix this bug is a bit longer than 2 lines"

 

Please have whatever devs are left over there take a look at Sklo's suggested easy fix. 

 

It's not really clear why we need to wait over three years for a five minute fix.

 

[Sklo:D 2,210]

bump

Edited June 28, 2019 by Sklo:D

[Sklo:D 2,210]

bump

[Sklo:D 2,210]

bump

[Sklo:D 2,210]

On 6/14/2019 at 11:03 PM, Sklo:D said:

In the end my solution to fix this bug is a bit longer than 2 lines but it is still simple enough and should fix the problems. I couldn't test this, so it is a guess, but I don't see any other possibility how the bug could be caused. Maybe something to check for @Keenanor @Budda

 

 

So I would try to fix this bug like that:

 

PlayerTransfer.class

public static final void sendItem(final Item item, final DataOutputStream dos, final boolean dragged) throws UnsupportedEncodingException, IOException {

//.....

        dos.writeBoolean(item.getLocked());
        dos.writeLong(item.getLockId());
        if (item.getLockId() != -10L) {
            if (item.isHollow()) {

                Item lock;
                try {
                    lock = Items.getItem(item.getLockId());
                    dos.writeBoolean(true);
                    sendItem(lock, dos, dragged);
                }
                catch (NoSuchItemException ex) {
                          try {

                              //Lock not loaded yet, loading...
                             lock = new DbItem(itemid);
                          }
                         catch (NoSuchItemException nsi) {
                              try {

                                    //Lock frozen, loading....
                                   lock = new DbItem(itemid, true);
                             }
                             catch (NoSuchItemException nsi2) {
                                  PlayerTransfer.logger.log(Level.WARNING, "Lock with the id "+lock.getId()+" of item "+item.getId()+" not found.");
                             }
                     }              
                }

                if(lock != null) {

                   Items.putItem(lock); //optional since the item will leave the server 

                   dos.writeBoolean(true);
                   sendItem(lock, dos, dragged);

                }

                else {

                   dos.writeBoolean(false);

                }
            }
            else {
                dos.writeBoolean(false);

                PlayerTransfer.logger.log(Level.WARNING, "Item "+item.getId()+" not hollow but locked.");
            }
        }

//....

 

Also improving Items.getItem(...) so it is actually safe and checking every possibility could be something which wouldn't be bad.

 

Bump apply my fix

[Keenan 5,539]

I've applied this fix, it will be in testing to ensure it doesn't muck with transfers.

 

My main concern is that this assumes the lock still indeed exists, but the logging will at least help us understand more if the fix proves to be less effective.

[Keenan 5,539]

On 6/13/2019 at 5:07 PM, Sklo:D said:

 

Any idea what is delaying the bug fix?

 

On 6/14/2019 at 6:27 PM, Brash_Endeavors said:

this was first posted three years ago  :   Posted June 6, 2016

 

>"my solution to fix this bug is a bit longer than 2 lines"

 

Please have whatever devs are left over there take a look at Sklo's suggested easy fix. 

 

It's not really clear why we need to wait over three years for a five minute fix.

 

 

One major reason that a bug like this doesn't get attention is that it deals with the transfer code.

 

Our transfer code is a huge pain point that we're painfully aware of, and to do much about it would require effort that we simply don't have the resources to commit to. To put it plainly, one false move in the transfer code and character data could be lost on transfer. That means people are very much against touching this code. Nearly every time someone does get brave there ends up being an issue requiring a hot fix. I have some experience with this code and I've also done recoveries when this code has gone wrong so I dared to touch it today.

 

Sorry it took so long.

[Sklo:D 2,210]

1 hour ago, Keenan said:

 

One major reason that a bug like this doesn't get attention is that it deals with the transfer code.

 

Our transfer code is a huge pain point that we're painfully aware of, and to do much about it would require effort that we simply don't have the resources to commit to. To put it plainly, one false move in the transfer code and character data could be lost on transfer. That means people are very much against touching this code. Nearly every time someone does get brave there ends up being an issue requiring a hot fix. I have some experience with this code and I've also done recoveries when this code has gone wrong so I dared to touch it today.

 

Sorry it took so long.

 

Btw. code is inspired by the #loadItem command functionality, so as long as #loadItem works this solution will also work. I never saw #loadItem fail.

 

And yeah it is NOT tested, so test it ^^

Edited July 19, 2019 by Sklo:D

[Keenan 5,539]

56 minutes ago, Sklo:D said:

 

Btw. code is inspired by the #loadItem command functionality, so as long as #loadItem works this solution will also work. I never saw #loadItem fail.

 

And yeah it is NOT tested, so test it ^^

 

My hair is already going gray... it shall be tested. lol

 

Every time I hear "Why is it telling me to create an account" I can hear another gray hair pop up.

[Keenan 5,539]

So this proposed fix didn't do the trick.

 

I think it fixed part of the issue, and I can now verify that the lock *is* being sent. I went beyond and added this proposal (tweaked, of course) to a getLock() method. I then had DbItem's getLockId() try to fetch the lock first before defaulting to the ID set on the item itself. This way any time anything tried to access the lock ID, we'd see an error if it couldn't load it - and it would load it if it could.

 

Few odd things now:

- DbItem's load method appears to be deprecated based on a logger added way back when by Erik. Not sure why and he's not here anymore to ask.

- Even though I can confirm that the lock is being created in the items table, the lock data in the lock table, and that the lock is being loaded by getLock... it still says there's no lock.

 

Definitely not an easy bug to step on here.

[Keenan 5,539]

An interesting development that came out of this is that locked containers inside a boat's hold cross just fine. Adding that here so perhaps people can use it as a work-around while we work on this issue.

[Sklo:D 2,210]

On 7/19/2019 at 8:26 PM, Keenan said:

So this proposed fix didn't do the trick.

 

I think it fixed part of the issue, and I can now verify that the lock *is* being sent. I went beyond and added this proposal (tweaked, of course) to a getLock() method. I then had DbItem's getLockId() try to fetch the lock first before defaulting to the ID set on the item itself. This way any time anything tried to access the lock ID, we'd see an error if it couldn't load it - and it would load it if it could.

 

Few odd things now:

- DbItem's load method appears to be deprecated based on a logger added way back when by Erik. Not sure why and he's not here anymore to ask.

- Even though I can confirm that the lock is being created in the items table, the lock data in the lock table, and that the lock is being loaded by getLock... it still says there's no lock.

 

Definitely not an easy bug to step on here.

 

Well without the fix the lock isn't even transfered to the other server. 

So it is an improvement, to track this further down I would be in need of the current code. I am optimistic as I don't think there is much left to fix, likely just some small problem.

[Anarres 246]

Is this on test server or live?

[John 254]

Reproduced on a small chest whilst crossing from Affliction to Elevation. No lock in examine, still getting  this message when trying to shove items into it: "The target item is locked but the lock could not be found. Rejecting move request."

[Ehizellbob 885]

Had this issue on a small chest also after crossing servers from Xanadu to Release. Fixed itself after server restart, a little annoying until that time since I couldn't access anything inside