getRoom in TArea causes seg fault on map creation

[Chris]

Creating a new map with this code on the latest development branch causes this backtrace:

0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
1 TMap::setExit TMap.cpp 304 0x5281ca
2 TLuaInterpreter::setExit TLuaInterpreter.cpp 6650 0x492075
3 ?? C:\mingw32\lib\lua51.dll 0x66d862e0
4 ?? C:\mingw32\lib\lua51.dll 0x66d8ffa6
5 ?? C:\mingw32\lib\lua51.dll 0x66d866e0
6 ?? C:\mingw32\lib\lua51.dll 0x66d81590
7 ?? C:\mingw32\lib\lua51.dll 0x66d85ad6
8 ?? C:\mingw32\lib\lua51.dll 0x66d86843
9 ?? C:\mingw32\lib\lua51.dll 0x66d82870
10 TLuaInterpreter::compile TLuaInterpreter.cpp 9649 0x49ce7a
11 TScript::compileScript TScript.cpp 150 0x4b04d7
12 TScript::setScript TScript.cpp 143 0x4b048e
13 dlgTriggerEditor::saveScript dlgTriggerEditor.cpp 4282 0x456ad9
14 dlgTriggerEditor::slot_saveScriptAfterEdit dlgTriggerEditor.cpp 4178 0x456799
15 dlgTriggerEditor::slot_save_edit dlgTriggerEditor.cpp 6932 0x46a642
16 dlgTriggerEditor::qt_static_metacall moc_dlgTriggerEditor.cpp 509 0x57ecd9
17 QMetaObject::activate qobject.cpp 3479 0x4c94cd0
18 QMetaObject::activate qobject.cpp 3354 0x4c94672
19 QAction::triggered moc_qaction.cpp 356 0x11674669
20 QAction::activate qaction.cpp 1175 0x11673c1e
... <More>

code:

-------------------------------------------------
-- Put your Lua functions here. --
-- --
-- Note that you can also use external Scripts --
-------------------------------------------------
function makeMap()
id = 0
for i=0,100 do
 for j=0, 100 do
  addRoom(id)
  --setRoomArea(id,0) --uncomment this and remove below call to avoid seg faults
  setRoomName(id, tostring(id))
  setRoomCoordinates(id, i, j, 0)
  setExit(id-1,id,1)
  setExit(id,id-1,2)
  setRoomArea(id,0)
  id = id+1
 end
end
end

function makeLabels()
 id=1
 for i=0,100 do
  for j=0, 100 do
   --display(id)
   x,y,z = getRoomCoordinates(id)
   createMapLabel(0,tostring(id),x,y,z,0,255,0,0,0,0)
   id = id+1
  end
 end
end

--makeMap()
--makeLabels()
centerview(1)

uncomment makeMap and it crashes. It fails when id is 1 as well, so it isn't because of the i-1 setExit bit. It's caused by mpRoomDB not being initialized because there is no area created. Even post creation, the room needs to be assigned to an area first before any calls to mpRoomDB are called, else it will seg fault. It seems like we need some 'void' area that has no name and is just an abyss for rooms waiting to be allocated to avoid breaking scripts.

[Vadim Peretokin (vperetokin)]

Yeah I agree. We should be able to create rooms, not have to place them
into an area - setup their properties and then add it to one.

[Stephen Lyons (slysven)]

On 17/09/13 23:59, Vadim Peretokin wrote:
> Yeah I agree. We should be able to create rooms, not have to place them
> into an area - setup their properties and then add it to one.
>

    Ah, I recently was adding some rooms to an old map to test some
mapping code. I had an empty area and although through the LUA
interface all the rooms seem to be in that area, the map drawing code
was treating some as being out of the area and not drawing exits/routes
between them properly when I added them later - the problem did not seem
to be fixed until after the map was saved and reloaded. I'd like to
know what was wrong (if anything). At a wild guess I'd say that some
structures are not being amended with new data as rooms are added. I
vaguely recall that some of the mapping data (the levels or "ebenung"?
tables) may not be being updated as rooms are inserted/deleted.

    I added into my code a LUA function to create a room (with the next
available RoomID) in an area with given areaID, coordinates and room
name but am I missing something in the room creation process?

    Also, do we not have an area with ID = 0 that always exists - would
it be safe to use that as an implicitly unnamed or default area (perhaps
it should have a fixed name of something like "_unnamed" so that it
always(?) is the first area in the mapper area selection list?) I can't
recall whether that area is actually displayable as a map anyway? If it
was would this still be problematic for inexperienced users starting out
building a fresh map and then wanting to name the first area they
created if they tried to rename such a default (un-renameable) area
rather than using the GUI or LUA code to move their rooms en-block to a
new area?

    I must check also whether it is allowed to add a room to an area
without giving any coordinates - I guess if it *is* you get a load of
rooms piling up at (0,0,0) which can be awkward to sort out later... 8-)

    Possible all-in-one room creation LUA function:
(Header file and LUA function registration not shown):
// createRoom( AreaID, RoomName, x, y, z )
// Returns new room ID, or nil on failure
// Convenience function to make a NEW room in a given area
int TLuaInterpreter::createRoom( lua_State *L )
{
    int areaId, x, y, z;
    Host * pHost = TLuaInterpreter::luaInterpreterMap[L];
    string name;
    TArea * pA;

    if(! pHost || ! pHost->mpMap || ! pHost->mpMap->mpRoomDB)
    {
        lua_pushstring( L, "createRoom: internal failure" );
        lua_error( L );
        return 1;
    }

    if( lua_isnumber( L, 1 ) )
    {
        areaId = lua_tonumber( L, 1 );
        pA = pHost->mpMap->mpRoomDB->getArea( areaId );
        if( !pA )
        {
            lua_pushstring( L, "createRoom: invalid areaId value" );
            lua_error( L );
            return 1;
        }
    }
    else
    {
        lua_pushstring( L, "createRoom: wrong argument(1) type" );
        lua_error( L );
        return 1;
    }

    if( lua_isstring( L, 2 ) )
    {
        name = lua_tostring( L, 2 );
    }
    else
    {
        lua_pushstring( L, "createRoom: wrong argument(2) type" );
        lua_error( L );
        return 1;
    }

    if( lua_isnumber( L...

[Chris (chrismudlet)]

You're correct that 0 can be used as the placeholder. All areas start at 1
(now at least). If this is historically true, it is a valid choice. I
don't think an all-in-one is a good idea personally since it makes us less
modular in the future.

On Wed, Sep 18, 2013 at 5:07 PM, Stephen Lyons
<email address hidden>wrote:

> On 17/09/13 23:59, Vadim Peretokin wrote:
> > Yeah I agree. We should be able to create rooms, not have to place them
> > into an area - setup their properties and then add it to one.
> >
>
> Ah, I recently was adding some rooms to an old map to test some
> mapping code. I had an empty area and although through the LUA
> interface all the rooms seem to be in that area, the map drawing code
> was treating some as being out of the area and not drawing exits/routes
> between them properly when I added them later - the problem did not seem
> to be fixed until after the map was saved and reloaded. I'd like to
> know what was wrong (if anything). At a wild guess I'd say that some
> structures are not being amended with new data as rooms are added. I
> vaguely recall that some of the mapping data (the levels or "ebenung"?
> tables) may not be being updated as rooms are inserted/deleted.
>
> I added into my code a LUA function to create a room (with the next
> available RoomID) in an area with given areaID, coordinates and room
> name but am I missing something in the room creation process?
>
> Also, do we not have an area with ID = 0 that always exists - would
> it be safe to use that as an implicitly unnamed or default area (perhaps
> it should have a fixed name of something like "_unnamed" so that it
> always(?) is the first area in the mapper area selection list?) I can't
> recall whether that area is actually displayable as a map anyway? If it
> was would this still be problematic for inexperienced users starting out
> building a fresh map and then wanting to name the first area they
> created if they tried to rename such a default (un-renameable) area
> rather than using the GUI or LUA code to move their rooms en-block to a
> new area?
>
> I must check also whether it is allowed to add a room to an area
> without giving any coordinates - I guess if it *is* you get a load of
> rooms piling up at (0,0,0) which can be awkward to sort out later... 8-)
>
>
> Possible all-in-one room creation LUA function:
> (Header file and LUA function registration not shown):
> // createRoom( AreaID, RoomName, x, y, z )
> // Returns new room ID, or nil on failure
> // Convenience function to make a NEW room in a given area
> int TLuaInterpreter::createRoom( lua_State *L )
> {
> int areaId, x, y, z;
> Host * pHost = TLuaInterpreter::luaInterpreterMap[L];
> string name;
> TArea * pA;
>
> if(! pHost || ! pHost->mpMap || ! pHost->mpMap->mpRoomDB)
> {
> lua_pushstring( L, "createRoom: internal failure" );
> lua_error( L );
> return 1;
> }
>
> if( lua_isnumber( L, 1 ) )
> {
> areaId = lua_tonumber( L, 1 );
> pA = pHost->mpMap->mpRoomDB->getArea( areaId );
> if( !pA )
> {
> lua_pushstring( L, "createRoom: invalid areaI...

[Vadim Peretokin (vperetokin)]

Lua isn't an acronym :)

You can add a room to an area without specifying the coordinates - it is a
two step process in the API. Do you mean to say whenever it will show in
the mapper if the coordinates aren't set?

What is your motivation behind the all in one function? We still need to
have the stepped process available. You might not know the rooms
coordinates upon wanting to create it, for example.

Internal failure isn't a good error message :) better to actually say what
the problem is - it will help us and the users. We haven't got proprietary
data we don't want to expose, anyway.

I've recently instituted a better format for error messages - search for
pushstringf. It's desirable to follow examples there as they are most
explanatory format we've got so far - and an error message that explains
the situation better is the more desirable one.

[Vadim Peretokin (vperetokin)]

What do you mean by all in one? The use of the 0 area?

[Chris (chrismudlet)]

By all in one I meant a single function that does everything. I think it
will end up being unwieldy and bloated as we add features -- especially if
we enhance a given feature that isn't last on the all-in-one function's
argument list.

The use of the 0 area was in reference to the 'void' area we can use for
rooms that are waiting for a home within an area. Do you have any real old
maps that you can check if area 0 is used?

On Wed, Sep 18, 2013 at 10:44 PM, Vadim Peretokin <email address hidden>wrote:

> What do you mean by all in one? The use of the 0 area?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1226558
>
> Title:
> getRoom in TArea causes seg fault on map creation
>
> Status in Mudlet the MUD client:
> New
>
> Bug description:
> Creating a new map with this code on the latest development branch
> causes this backtrace:
>
> 0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
> 1 TMap::setExit TMap.cpp 304 0x5281ca
> 2 TLuaInterpreter::setExit TLuaInterpreter.cpp 6650
> 0x492075
> 3 ?? C:\mingw32\lib\lua51.dll 0x66d862e0
> 4 ?? C:\mingw32\lib\lua51.dll 0x66d8ffa6
> 5 ?? C:\mingw32\lib\lua51.dll 0x66d866e0
> 6 ?? C:\mingw32\lib\lua51.dll 0x66d81590
> 7 ?? C:\mingw32\lib\lua51.dll 0x66d85ad6
> 8 ?? C:\mingw32\lib\lua51.dll 0x66d86843
> 9 ?? C:\mingw32\lib\lua51.dll 0x66d82870
> 10 TLuaInterpreter::compile TLuaInterpreter.cpp 9649
> 0x49ce7a
> 11 TScript::compileScript TScript.cpp 150 0x4b04d7
> 12 TScript::setScript TScript.cpp 143 0x4b048e
> 13 dlgTriggerEditor::saveScript dlgTriggerEditor.cpp 4282
> 0x456ad9
> 14 dlgTriggerEditor::slot_saveScriptAfterEdit
> dlgTriggerEditor.cpp 4178 0x456799
> 15 dlgTriggerEditor::slot_save_edit dlgTriggerEditor.cpp
> 6932 0x46a642
> 16 dlgTriggerEditor::qt_static_metacall moc_dlgTriggerEditor.cpp
> 509 0x57ecd9
> 17 QMetaObject::activate qobject.cpp 3479 0x4c94cd0
> 18 QMetaObject::activate qobject.cpp 3354 0x4c94672
> 19 QAction::triggered moc_qaction.cpp 356 0x11674669
> 20 QAction::activate qaction.cpp 1175 0x11673c1e
> ... <More>
>
>
> code:
>
> -------------------------------------------------
> -- Put your Lua functions here. --
> -- --
> -- Note that you can also use external Scripts --
> -------------------------------------------------
> function makeMap()
> id = 0
> for i=0,100 do
> for j=0, 100 do
> addRoom(id)
> --setRoomArea(id,0) --uncomment this and remove below call
> to avoid seg faults
> setRoomName(id, tostring(id))
> setRoomCoordinates(id, i, j, 0)
> setExit(id-1,id,1)
> setExit(id,id-1,2)
> setRoo...

[Vadim Peretokin (vperetokin)]

Yeah, I am in agreement there - I wouldn't go down the path of all in one.
We could add it for conveniences sake - for example we have the all in one
function to creating miniconsoles, but we also have the option of doing it
step by step. However with Lua's ordered arguments (unless you're using
tables, which does not make for an efficient API) then tacking everything
to the end of a function becomes unwieldy.

I've never used area 0 really. It's mostly annoying in the fact that it
shows up in the list of areas the map has and in the mapper widget as a
blank entry. I'd support hiding it from the API (partially - so we don't
see it, but can use it) and the user (not showing in the mapper widget) and
making use of it as the temporary void storage.

[Chris (chrismudlet)]

Heiko,

Do you have any uncommitted work to the mapper that would make this a waste
of time to pursue at the moment?

On Thu, Sep 19, 2013 at 5:45 PM, Vadim Peretokin <email address hidden>wrote:

> Yeah, I am in agreement there - I wouldn't go down the path of all in one.
> We could add it for conveniences sake - for example we have the all in one
> function to creating miniconsoles, but we also have the option of doing it
> step by step. However with Lua's ordered arguments (unless you're using
> tables, which does not make for an efficient API) then tacking everything
> to the end of a function becomes unwieldy.
>
> I've never used area 0 really. It's mostly annoying in the fact that it
> shows up in the list of areas the map has and in the mapper widget as a
> blank entry. I'd support hiding it from the API (partially - so we don't
> see it, but can use it) and the user (not showing in the mapper widget) and
> making use of it as the temporary void storage.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1226558
>
> Title:
> getRoom in TArea causes seg fault on map creation
>
> Status in Mudlet the MUD client:
> New
>
> Bug description:
> Creating a new map with this code on the latest development branch
> causes this backtrace:
>
> 0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
> 1 TMap::setExit TMap.cpp 304 0x5281ca
> 2 TLuaInterpreter::setExit TLuaInterpreter.cpp 6650
> 0x492075
> 3 ?? C:\mingw32\lib\lua51.dll 0x66d862e0
> 4 ?? C:\mingw32\lib\lua51.dll 0x66d8ffa6
> 5 ?? C:\mingw32\lib\lua51.dll 0x66d866e0
> 6 ?? C:\mingw32\lib\lua51.dll 0x66d81590
> 7 ?? C:\mingw32\lib\lua51.dll 0x66d85ad6
> 8 ?? C:\mingw32\lib\lua51.dll 0x66d86843
> 9 ?? C:\mingw32\lib\lua51.dll 0x66d82870
> 10 TLuaInterpreter::compile TLuaInterpreter.cpp 9649
> 0x49ce7a
> 11 TScript::compileScript TScript.cpp 150 0x4b04d7
> 12 TScript::setScript TScript.cpp 143 0x4b048e
> 13 dlgTriggerEditor::saveScript dlgTriggerEditor.cpp 4282
> 0x456ad9
> 14 dlgTriggerEditor::slot_saveScriptAfterEdit
> dlgTriggerEditor.cpp 4178 0x456799
> 15 dlgTriggerEditor::slot_save_edit dlgTriggerEditor.cpp
> 6932 0x46a642
> 16 dlgTriggerEditor::qt_static_metacall moc_dlgTriggerEditor.cpp
> 509 0x57ecd9
> 17 QMetaObject::activate qobject.cpp 3479 0x4c94cd0
> 18 QMetaObject::activate qobject.cpp 3354 0x4c94672
> 19 QAction::triggered moc_qaction.cpp 356 0x11674669
> 20 QAction::activate qaction.cpp 1175 0x11673c1e
> ... <More>
>
>
> code:
>
> -------------------------------------------------
> -- Put your Lua functions here. --
> -- --
> -- Note that you can also use external Scripts --
> --------------------------------------------...

[Heiko (koehnheiko)]

No, I don't think so.

Am 20.09.2013 16:24, schrieb Chris:
> Heiko,
>
> Do you have any uncommitted work to the mapper that would make this a waste
> of time to pursue at the moment?
>
>
> On Thu, Sep 19, 2013 at 5:45 PM, Vadim Peretokin <email address hidden>wrote:
>
>> Yeah, I am in agreement there - I wouldn't go down the path of all in one.
>> We could add it for conveniences sake - for example we have the all in one
>> function to creating miniconsoles, but we also have the option of doing it
>> step by step. However with Lua's ordered arguments (unless you're using
>> tables, which does not make for an efficient API) then tacking everything
>> to the end of a function becomes unwieldy.
>>
>> I've never used area 0 really. It's mostly annoying in the fact that it
>> shows up in the list of areas the map has and in the mapper widget as a
>> blank entry. I'd support hiding it from the API (partially - so we don't
>> see it, but can use it) and the user (not showing in the mapper widget) and
>> making use of it as the temporary void storage.
>>
>> --
>> You received this bug notification because you are subscribed to the bug
>> report.
>> https://bugs.launchpad.net/bugs/1226558
>>
>> Title:
>> getRoom in TArea causes seg fault on map creation
>>
>> Status in Mudlet the MUD client:
>> New
>>
>> Bug description:
>> Creating a new map with this code on the latest development branch
>> causes this backtrace:
>>
>> 0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
>> 1 TMap::setExit TMap.cpp 304 0x5281ca
>> 2 TLuaInterpreter::setExit TLuaInterpreter.cpp 6650
>> 0x492075
>> 3 ?? C:\mingw32\lib\lua51.dll 0x66d862e0
>> 4 ?? C:\mingw32\lib\lua51.dll 0x66d8ffa6
>> 5 ?? C:\mingw32\lib\lua51.dll 0x66d866e0
>> 6 ?? C:\mingw32\lib\lua51.dll 0x66d81590
>> 7 ?? C:\mingw32\lib\lua51.dll 0x66d85ad6
>> 8 ?? C:\mingw32\lib\lua51.dll 0x66d86843
>> 9 ?? C:\mingw32\lib\lua51.dll 0x66d82870
>> 10 TLuaInterpreter::compile TLuaInterpreter.cpp 9649
>> 0x49ce7a
>> 11 TScript::compileScript TScript.cpp 150 0x4b04d7
>> 12 TScript::setScript TScript.cpp 143 0x4b048e
>> 13 dlgTriggerEditor::saveScript dlgTriggerEditor.cpp 4282
>> 0x456ad9
>> 14 dlgTriggerEditor::slot_saveScriptAfterEdit
>> dlgTriggerEditor.cpp 4178 0x456799
>> 15 dlgTriggerEditor::slot_save_edit dlgTriggerEditor.cpp
>> 6932 0x46a642
>> 16 dlgTriggerEditor::qt_static_metacall moc_dlgTriggerEditor.cpp
>> 509 0x57ecd9
>> 17 QMetaObject::activate qobject.cpp 3479 0x4c94cd0
>> 18 QMetaObject::activate qobject.cpp 3354 0x4c94672
>> 19 QAction::triggered moc_qaction.cpp 356 0x11674669
>> 20 QAction::activate qaction.cpp 1175 0x11673c1e
>> ... <More>
>>
>>
>> code:
>>
>> -------------------------------------------------
>> -- Put your Lua functions here...

[Stephen Lyons (slysven)]

Ah, one of those patches I sent you Heiko overhauls the room exit
dialogue though I don't think it'll be hit by anything that revises the
TArea code.

I notice from the recent git master branch that setting a normal room's
area to 0 seems to be permitted by the lua code but it really messes
things up for that room - the mapper can't seem to decide which area the
room is in and I think the mapper code draws both "in area" and "out of
area" exits to (or from?) that room. Using the lua command to then move
the room to another area (or possibly back to the original one) fails
with a "room already exists" sort of message (sorry can't be precise as
don't have development pc booted as I write). I think that trying to
move the room (original area)->"0"->(different area) ends up with it
being duplicated and that does "not work well" 8-/ in the current code...!

Inspecting the code - I'm not clear how TRoom::setArea( int _areaID )
called from TMap::setRoomArea( int _areaID ) called from either
TLuaInterpreter::setRoomArea() or from somewhere in T2DMap.cpp behaves
if the room concerned has already been allocated to an area. I can't
see where the room is removed from the TArea(*)->rooms for the *old*
area (or its limits recalculated). I may be missing something of course
but I'm suspicious!

On 23/09/13 07:22, Heiko wrote:
> No, I don't think so.
>
> Am 20.09.2013 16:24, schrieb Chris:
>> Heiko,
>>
>> Do you have any uncommitted work to the mapper that would make this a waste
>> of time to pursue at the moment?
>>
>>
>> On Thu, Sep 19, 2013 at 5:45 PM, Vadim Peretokin <email address hidden>wrote:
>>
>>> Yeah, I am in agreement there - I wouldn't go down the path of all in one.
>>> We could add it for conveniences sake - for example we have the all in one
>>> function to creating miniconsoles, but we also have the option of doing it
>>> step by step. However with Lua's ordered arguments (unless you're using
>>> tables, which does not make for an efficient API) then tacking everything
>>> to the end of a function becomes unwieldy.
>>>
>>> I've never used area 0 really. It's mostly annoying in the fact that it
>>> shows up in the list of areas the map has and in the mapper widget as a
>>> blank entry. I'd support hiding it from the API (partially - so we don't
>>> see it, but can use it) and the user (not showing in the mapper widget) and
>>> making use of it as the temporary void storage.
>>>
>>> --
>>> You received this bug notification because you are subscribed to the bug
>>> report.
>>> https://bugs.launchpad.net/bugs/1226558
>>>
>>> Title:
>>> getRoom in TArea causes seg fault on map creation
>>>
>>> Status in Mudlet the MUD client:
>>> New
>>>
>>> Bug description:
>>> Creating a new map with this code on the latest development branch
>>> causes this backtrace:
>>>
>>> 0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
>>> 1 TMap::setExit TMap.cpp 304 0x5281ca
>>> 2 TLuaInterpreter::setExit TLuaInterpreter.cpp 6650
>>> 0x492075
>>> 3 ?? C:\mingw32\lib\lua51.dll 0x66d862e0
>>> 4 ?? C:\mingw32\lib\lua51.dll 0x66d8ffa6
>>...

[Chris (chrismudlet)]

Would it be correct then to classify rooms with area 0 as a bug then? What
repercussions would there be if we disallowed the 0 area index, and use -1
as a 'void' area? The import code could re-classify area 0 if it exists to
deal with compatibility.

On Mon, Sep 23, 2013 at 10:48 PM, Stephen Lyons
<email address hidden>wrote:

> Ah, one of those patches I sent you Heiko overhauls the room exit
> dialogue though I don't think it'll be hit by anything that revises the
> TArea code.
>
> I notice from the recent git master branch that setting a normal room's
> area to 0 seems to be permitted by the lua code but it really messes
> things up for that room - the mapper can't seem to decide which area the
> room is in and I think the mapper code draws both "in area" and "out of
> area" exits to (or from?) that room. Using the lua command to then move
> the room to another area (or possibly back to the original one) fails
> with a "room already exists" sort of message (sorry can't be precise as
> don't have development pc booted as I write). I think that trying to
> move the room (original area)->"0"->(different area) ends up with it
> being duplicated and that does "not work well" 8-/ in the current code...!
>
> Inspecting the code - I'm not clear how TRoom::setArea( int _areaID )
> called from TMap::setRoomArea( int _areaID ) called from either
> TLuaInterpreter::setRoomArea() or from somewhere in T2DMap.cpp behaves
> if the room concerned has already been allocated to an area. I can't
> see where the room is removed from the TArea(*)->rooms for the *old*
> area (or its limits recalculated). I may be missing something of course
> but I'm suspicious!
>
>
> On 23/09/13 07:22, Heiko wrote:
> > No, I don't think so.
> >
> > Am 20.09.2013 16:24, schrieb Chris:
> >> Heiko,
> >>
> >> Do you have any uncommitted work to the mapper that would make this a
> waste
> >> of time to pursue at the moment?
> >>
> >>
> >> On Thu, Sep 19, 2013 at 5:45 PM, Vadim Peretokin <<email address hidden>
> >wrote:
> >>
> >>> Yeah, I am in agreement there - I wouldn't go down the path of all in
> one.
> >>> We could add it for conveniences sake - for example we have the all in
> one
> >>> function to creating miniconsoles, but we also have the option of
> doing it
> >>> step by step. However with Lua's ordered arguments (unless you're using
> >>> tables, which does not make for an efficient API) then tacking
> everything
> >>> to the end of a function becomes unwieldy.
> >>>
> >>> I've never used area 0 really. It's mostly annoying in the fact that it
> >>> shows up in the list of areas the map has and in the mapper widget as a
> >>> blank entry. I'd support hiding it from the API (partially - so we
> don't
> >>> see it, but can use it) and the user (not showing in the mapper
> widget) and
> >>> making use of it as the temporary void storage.
> >>>
> >>> --
> >>> You received this bug notification because you are subscribed to the
> bug
> >>> report.
> >>> https://bugs.launchpad.net/bugs/1226558
> >>>
> >>> Title:
> >>> getRoom in TArea causes seg fault on map creation
> >>>
> >>> Status in Mudlet the MUD client:
> >>> New
> >>>
> >>> Bug description:
> >>> Cr...

[Vadim Peretokin (vperetokin)]

My area ID 0 is unused, wouldn't break anything here. Somehow, however,
"unlocking" the area caused a lot of crashes on Windows, and it was "fixed"
by re-locking it:
http://forums.achaea.com/discussion/comment/102118/#Comment_102118

Even though the area locking code simply calls lockRoom on every room from
getAreaRooms(0), which returns nil, so it would not have been doing
anything...

It's anectodal evidence, but area 0 caused quite a problem for a short bit.

[Chris (chrismudlet)]

So I think the best solution is to nix the use of area 0, have -1 be a void area, and enumeration begin at 1. This doesn't appear to break any scripts, as using area 0 actually causes scripts to break. If perusing the code supports this, I'll put in this fix.

[Stephen Lyons (slysven)]

The thing about area 0 seems to be we don't seem to actually explicitly
create it! Also the code to move rooms to an area does not seem to
update the old area to take the room FROM it - we get away with this at
mostly at the moment because I guess the typical usage is to create a
room and then put it into the desired area and relatively rarely does
rooms get moved to a new area.

I'd suggest ensuring that an area 0 is created with new maps, and that
during map auditing of existing ones, we add area 0 and all rooms with
area = 0 to that area, also I'd be minded to check that each room only
appears in the "rooms" of one area. If we increment the map version at
this point the user will only be hit by these extra checks once per map
file.

In general adding a room should auto-magically place it in area 0. Also
TArea.cpp need a removeRoom method which will be used both on room
deletion and movement to a different area. Of course calls that add or
delete rooms from an area should also cause an update to the min/maxs
and the ebenes (levels?) for that area - which I am not sure is
happening universally at the present.

Right, I'm going to brew a fresh cup of coffee and take a look at trying
to coding this unless someone else has a solution already worked out...

On 27/09/13 16:50, Chris wrote:
> So I think the best solution is to nix the use of area 0, have -1 be a
> void area, and enumeration begin at 1. This doesn't appear to break any
> scripts, as using area 0 actually causes scripts to break. If perusing
> the code supports this, I'll put in this fix.
>

[Vadim Peretokin (vperetokin)]

On Sat, Sep 28, 2013 at 3:53 AM, Stephen Lyons
<email address hidden>wrote:

> The thing about area 0 seems to be we don't seem to actually explicitly
> create it! Also the code to move rooms to an area does not seem to
> update the old area to take the room FROM it - we get away with this at
> mostly at the moment because I guess the typical usage is to create a
> room and then put it into the desired area and relatively rarely does
> rooms get moved to a new area.
>
>
Do you mean that the room gets left over in the old area? This doesn't seem
to be the case right now... which case do you speak of that would break it
here?

There have been a couple of people recently who started writing mapper
scripts and have been crashing Mudlet quite often - so I'm suspicious here.

> I'd suggest ensuring that an area 0 is created with new maps, and that
> during map auditing of existing ones, we add area 0 and all rooms with
> area = 0 to that area, also I'd be minded to check that each room only
> appears in the "rooms" of one area. If we increment the map version at
> this point the user will only be hit by these extra checks once per map
> file.
>
>
I think you're using area 0 in several concepts here. Otherwise "I'd
suggest ensuring that an area 0 is created with new maps, and that during
map auditing of existing ones, we add area 0 and all rooms with area = 0 to
that area [area 0]" doens't make sense. Do you mean rooms with an unset
area... ? But that doesn't fix the "we add area 0 to area 0" logical
peculiarity.

> In general adding a room should auto-magically place it in area 0. Also
> TArea.cpp need a removeRoom method which will be used both on room
> deletion and movement to a different area. Of course calls that add or
> delete rooms from an area should also cause an update to the min/maxs
> and the ebenes (levels?) for that area - which I am not sure is
> happening universally at the present.
>
>
That'd make sense.

> Right, I'm going to brew a fresh cup of coffee and take a look at trying
> to coding this unless someone else has a solution already worked out...
>
>
I'd be a wary here, we'd better get Heikos sign-off on this. He's never
documented his design in this so you wouldn't know what else might you
break without consulting him first (and breaking the design or going
against his philosophy is something that could happen, and result in wasted
effort).

> On 27/09/13 16:50, Chris wrote:
> > So I think the best solution is to nix the use of area 0, have -1 be a
> > void area, and enumeration begin at 1. This doesn't appear to break any
> > scripts, as using area 0 actually causes scripts to break. If perusing
> > the code supports this, I'll put in this fix.
> >
>
> --
> You received this bug notification because you are a member of Mudlet
> Makers, which is subscribed to Mudlet.
> https://bugs.launchpad.net/bugs/1226558
>
> Title:
> getRoom in TArea causes seg fault on map creation
>
> Status in Mudlet the MUD client:
> New
>
> Bug description:
> Creating a new map with this code on the latest development branch
> causes this backtrace:
>
> 0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
> ...

[Stephen Lyons (slysven)]

On 28/09/13 00:50, Vadim Peretokin wrote:
> On Sat, Sep 28, 2013 at 3:53 AM, Stephen Lyons
> <email address hidden>wrote:
>
>> The thing about area 0 seems to be we don't seem to actually explicitly
>> create it! Also the code to move rooms to an area does not seem to
>> update the old area to take the room FROM it - we get away with this at
>> mostly at the moment because I guess the typical usage is to create a
>> room and then put it into the desired area and relatively rarely does
>> rooms get moved to a new area.
>>
>>
> Do you mean that the room gets left over in the old area? This doesn't seem
> to be the case right now... which case do you speak of that would break it
> here?
>
> There have been a couple of people recently who started writing mapper
> scripts and have been crashing Mudlet quite often - so I'm suspicious here.
>
>
>> I'd suggest ensuring that an area 0 is created with new maps, and that
>> during map auditing of existing ones, we add area 0 and all rooms with
>> area = 0 to that area, also I'd be minded to check that each room only
>> appears in the "rooms" of one area. If we increment the map version at
>> this point the user will only be hit by these extra checks once per map
>> file.
>>
>>
> I think you're using area 0 in several concepts here. Otherwise "I'd
> suggest ensuring that an area 0 is created with new maps, and that during
> map auditing of existing ones, we add area 0 and all rooms with area = 0 to
> that area [area 0]" doens't make sense. Do you mean rooms with an unset
> area... ? But that doesn't fix the "we add area 0 to area 0" logical
> peculiarity.

I think I needed another comma ',' or a couple of extra words:
>"I'd suggest ensuring that an area 0 is created with new maps, and
> that during map auditing of existing ones, we add area 0*[,]* and
> [then] all rooms with area = 0 to [that] area [area 0]"

The TRoom class has an int called "area" which is 0 in the constructor,
but gets set to the areaID as the room is added to the appropriate
TRoomDB's QMap<int, TArea *> "areas" member's "rooms" member - and
currently actually also gets set even if "area" is Not a valid areaID
value...

What I mean is to create an area 0 in TRoomDB areas if it does not
exist, then add any existing "orphan" room (not belonging to an area
already and thus their "area" value is zero) to that area during the map
auditing process. Probably because of that defect over setting the
TRoom area value I noticed I found with one test map I am using around
ten rooms that had disappeared from the map which had a non-zero and
valid value in that member but which were NOT in the corresponding
TArea's rooms list and had thus dropped off the map.

As I type I have got some updated auditing code that reports these
problems (and checks every room is in one and only one TArea rooms list,
and that the TRoom "area" value matches the "areaID" of the TArea
"rooms" list in which it appears). I need to do more tests with a map
file that has rooms with no area both with and without a predefined area
0 which is what I am just about to do...

>
>
>> In general adding a room should auto-magically place it in area 0. Also
>> TArea.cp...

[Vadim Peretokin (vperetokin)]

Aha, that makes sense. Yes, I agree - we need to do this.

[Chris (chrismudlet)]

Sounds good to me.

On Sun, Sep 29, 2013 at 7:56 PM, Vadim Peretokin <email address hidden>wrote:

> Aha, that makes sense. Yes, I agree - we need to do this.
>
> ** Changed in: mudlet
> Status: New => Confirmed
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1226558
>
> Title:
> getRoom in TArea causes seg fault on map creation
>
> Status in Mudlet the MUD client:
> Confirmed
>
> Bug description:
> Creating a new map with this code on the latest development branch
> causes this backtrace:
>
> 0 TArea::fast_ausgaengeBestimmen TArea.cpp 168 0x5128bb
> 1 TMap::setExit TMap.cpp 304 0x5281ca
> 2 TLuaInterpreter::setExit TLuaInterpreter.cpp 6650
> 0x492075
> 3 ?? C:\mingw32\lib\lua51.dll 0x66d862e0
> 4 ?? C:\mingw32\lib\lua51.dll 0x66d8ffa6
> 5 ?? C:\mingw32\lib\lua51.dll 0x66d866e0
> 6 ?? C:\mingw32\lib\lua51.dll 0x66d81590
> 7 ?? C:\mingw32\lib\lua51.dll 0x66d85ad6
> 8 ?? C:\mingw32\lib\lua51.dll 0x66d86843
> 9 ?? C:\mingw32\lib\lua51.dll 0x66d82870
> 10 TLuaInterpreter::compile TLuaInterpreter.cpp 9649
> 0x49ce7a
> 11 TScript::compileScript TScript.cpp 150 0x4b04d7
> 12 TScript::setScript TScript.cpp 143 0x4b048e
> 13 dlgTriggerEditor::saveScript dlgTriggerEditor.cpp 4282
> 0x456ad9
> 14 dlgTriggerEditor::slot_saveScriptAfterEdit
> dlgTriggerEditor.cpp 4178 0x456799
> 15 dlgTriggerEditor::slot_save_edit dlgTriggerEditor.cpp
> 6932 0x46a642
> 16 dlgTriggerEditor::qt_static_metacall moc_dlgTriggerEditor.cpp
> 509 0x57ecd9
> 17 QMetaObject::activate qobject.cpp 3479 0x4c94cd0
> 18 QMetaObject::activate qobject.cpp 3354 0x4c94672
> 19 QAction::triggered moc_qaction.cpp 356 0x11674669
> 20 QAction::activate qaction.cpp 1175 0x11673c1e
> ... <More>
>
>
> code:
>
> -------------------------------------------------
> -- Put your Lua functions here. --
> -- --
> -- Note that you can also use external Scripts --
> -------------------------------------------------
> function makeMap()
> id = 0
> for i=0,100 do
> for j=0, 100 do
> addRoom(id)
> --setRoomArea(id,0) --uncomment this and remove below call
> to avoid seg faults
> setRoomName(id, tostring(id))
> setRoomCoordinates(id, i, j, 0)
> setExit(id-1,id,1)
> setExit(id,id-1,2)
> setRoomArea(id,0)
> id = id+1
> end
> end
> end
>
> function makeLabels()
> id=1
> for i=0,100 do
> for j=0, 100 do
> --display(id)
> x,y,z = getRoomCoordinates(id)
> createMapLabel(0,tostring(id),x,y,z,0,255,0,0,...

[Chris (chrismudlet)]

In my repo, will push to Mudlet github after further testing