getExitStubs() returns the room ID instead of nil when there are no stubs

Bug #1261536 reported by Vadim Peretokin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mudlet
Fix Released
High
Chris

Bug Description

getExitStubs() returns the room ID instead of nil when there are no stubs . See http://forums.mudlet.org/viewtopic.php?f=7&t=4272&p=19841#p19840. This is just non-sensical and makes it difficult to work with as the code that this design forces isn't obvious (why would someone be checking for a number and not just nil for the lack of stubs?)

Revision history for this message
Stephen Lyons (slysven) wrote :

There does seem to be an error in all versions of the TLuaInterpreter::getExitStubs(...) method, Chris's commit on 2013-04-09 03:22:11 {63395a6c50f948a92a71df0e712b2476402792f1} does a small change in this area but does not address the fault that was present right back to the introduction. The attached patch will cure the problem.

Note that this is one of the table returning functions that has the issue of indexes starting at 0 rather than the LUA standard of 1!

Changed in mudlet:
status: New → Confirmed
importance: Undecided → High
Revision history for this message
Vadim Peretokin (vperetokin) wrote : Re: [Bug 1261536] Re: getExitStubs() returns the room ID instead of nil when there are no stubs

@0 index: yeah, it is. Unfortunately we'll break existing code that had to
have hacked around this issue if we fix it to use 1 index.

We should not let any other functions slip in that do this.

Revision history for this message
Chris (chrismudlet) wrote :

Sorry, c++ habits.

sent from my phone
On Dec 16, 2013 6:36 PM, "Vadim Peretokin" <email address hidden> wrote:

> @0 index: yeah, it is. Unfortunately we'll break existing code that had to
> have hacked around this issue if we fix it to use 1 index.
>
> We should not let any other functions slip in that do this.
>
> --
> You received this bug notification because you are subscribed to Mudlet.
> https://bugs.launchpad.net/bugs/1261536
>
> Title:
> getExitStubs() returns the room ID instead of nil when there are no
> stubs
>
> Status in Mudlet the MUD client:
> Confirmed
>
> Bug description:
> getExitStubs() returns the room ID instead of nil when there are no
> stubs . See
> http://forums.mudlet.org/viewtopic.php?f=7&t=4272&p=19841#p19840. This
> is just non-sensical and makes it difficult to work with as the code
> that this design forces isn't obvious (why would someone be checking
> for a number and not just nil for the lack of stubs?)
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mudlet/+bug/1261536/+subscriptions
>

Revision history for this message
Vadim Peretokin (vperetokin) wrote :

Not blaming anyone, I forgot it was you who added it (that's unproductive
to do anyway; fix and move on). We've got this case in more than one
function anyway - gotta be better and do code review. Sometimes you just
don't have the time though.

Revision history for this message
Stephen Lyons (slysven) wrote :

Actually, as a fix for the [0] issue, why not provide, e.g. a
getExitStubs1() - note the '1' suffix - for those few messed up cases
and correct the index issue in them. Going forward we can document
these case and direct new users to them, pointing out way we haven't
changed the originals to preserve functionality.

Anyone else remember the BASIC command "OPTION BASE" that some dialects
had to switch between arrays in that language starting from 0 or 1? 8-)

On 17/12/13 01:36, Chris wrote:
> Sorry, c++ habits.
>
> sent from my phone
> On Dec 16, 2013 6:36 PM, "Vadim Peretokin" <email address hidden> wrote:
>
>> @0 index: yeah, it is. Unfortunately we'll break existing code that had to
>> have hacked around this issue if we fix it to use 1 index.
>>
>> We should not let any other functions slip in that do this.
>>
>> --
>> You received this bug notification because you are subscribed to Mudlet.
>> https://bugs.launchpad.net/bugs/1261536
>>
>> Title:
>> getExitStubs() returns the room ID instead of nil when there are no
>> stubs
>>
>> Status in Mudlet the MUD client:
>> Confirmed
>>
>> Bug description:
>> getExitStubs() returns the room ID instead of nil when there are no
>> stubs . See
>> http://forums.mudlet.org/viewtopic.php?f=7&t=4272&p=19841#p19840. This
>> is just non-sensical and makes it difficult to work with as the code
>> that this design forces isn't obvious (why would someone be checking
>> for a number and not just nil for the lack of stubs?)
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/mudlet/+bug/1261536/+subscriptions
>>
>

Revision history for this message
Vadim Peretokin (vperetokin) wrote : Re: [Mudlet-makers] [Bug 1261536] Re: getExitStubs() returns the room ID instead of nil when there are no stubs

Not clean but it'll work and stop forcing the for i = 0 into code. I
approve.

Revision history for this message
Chris (chrismudlet) wrote :

It's not pretty but I think it's a consistent and user intuitive way to
handle all existing cases.
On Dec 16, 2013 9:37 PM, "Vadim Peretokin" <email address hidden> wrote:

> Not clean but it'll work and stop forcing the for i = 0 into code. I
> approve.
>
> --
> You received this bug notification because you are subscribed to Mudlet.
> https://bugs.launchpad.net/bugs/1261536
>
> Title:
> getExitStubs() returns the room ID instead of nil when there are no
> stubs
>
> Status in Mudlet the MUD client:
> Confirmed
>
> Bug description:
> getExitStubs() returns the room ID instead of nil when there are no
> stubs . See
> http://forums.mudlet.org/viewtopic.php?f=7&t=4272&p=19841#p19840. This
> is just non-sensical and makes it difficult to work with as the code
> that this design forces isn't obvious (why would someone be checking
> for a number and not just nil for the lack of stubs?)
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mudlet/+bug/1261536/+subscriptions
>

Revision history for this message
Chris (chrismudlet) wrote :
Changed in mudlet:
milestone: none → 3.0
status: Confirmed → Fix Committed
assignee: nobody → Chris (chrismudlet)
Changed in mudlet:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.