Obligatory "I ruined everything!" thread

[xoft]

Rev 629: Removed the deprecated cWorld:GetBlockEntity() function that didn't work anyway, added new enumerators instead:
cWorld:ForEachChestInChunk(ChunkX, ChunkZ, Callback)
- Calls the callback for each chest found in the chunk. Returns true if all chests enumerated, false if the callback returned true to abort enumeration

cWorld:ForEachFurnaceInChunk(ChunkX, ChunkZ, Callback)
- Calls the callback for each furnace found in the chunk. Returns true if all chests enumerated, false if the callback returned true to abort enumeration

cWorld:DoWithChestAt(BlockX, BlockY, BlockZ, Callback)
- If there is a chest at the specified coords, calls the Callback with the chest entity as the parameter and returns true. Returns false otherwise.

cWorld:DoWithFurnaceAt(BlockX, BlockY, BlockZ, Callback)
- If there is a furnace at the specified coords, calls the Callback with the furnace entity as the parameter and returns true. Returns false otherwise.

cWorld:GetSignLines(BlockX, BlockY, BlockZ)
- If there is a sign at the specified coords, returns true and the four lines (as a 5-tuplet return value); returns false if no such sign.

[xoft]

Rev 634:
Removed cItem:GetJson() and cItem:FromJson() from Lua API, those functions are for internal MCServer use only.

[xoft]

I guess we're making too many code changes for you to keep up

[Taugeshtu]

And I think I'm too lazy to keep up
Don't mind, one day I'll catch you up Keep doing good stuff

[xoft]

Rev 639:
Removed the cWorld:GetPlayer() function both from the API and from the C++ code altogether, it was unsafe. Use cWorld:ForEachPlayer() instead
Added the cWorld:DoWithPlayer() function to both C++ code and the Lua API. It takes two arguments, a player name and a callback to call. It works similar to ForEachPlayer(), but calls the callback only up to once, with the player with the specified name. Returns true if the player was found, false if not. Callback return value is ignored.

Here's an actual code snippet that works with DoWithPlayer():

Code:

function HandleKillPlayer(Split, InitiatorPlayer)
    -- InitiatorPlayer is valid throughout this function, even within the local function KillPlayer()
    local World = cRoot:Get():GetDefaultWorld()
    local PlayerName = Split[2] or ""
    if (PlayerName == "") then
        InitiatorPlayer:SendMessage(cChatColor.Green .. "Expected the player name to kill as the parameter")
        return true
    end

    local KillPlayer = function(Player)
        -- Player is valid only within this function, it cannot be stord and used later!
        Player:SendMessage("Incoming gift from " .. InitiatorPlayer:GetName() .. " to " .. PlayerName .. ": " .. (Split[3] or ""))
        Player:TakeDamage(100, InitiatorPlayer)
        HasFound = true
    end
    if (not(World:DoWithPlayer(PlayerName, KillPlayer))) then
        InitiatorPlayer:SendMessage(cChatColor.Green .. "Could not find player " .. PlayerName)
    end
    return true
end

[xoft]

Rev 642 adds a new interface, cBlockArea. It is to be used for querying an area of neighboring blocks together, so it's done faster.

Usage:
1, Create a cBlockArea object
2, Call its Read() function, specifying the coord ranges and the data types to read
3, Use the GetBlockXXX() functions to read individual block's data
4, Use the SetBlockXXX() function to write individual block's data. The writes are local only, they don't get back into the world.
5, (Not implemented yet!) call Write() function to write the changes back into World, all at once.

Use the cBlockArea object whenever you need to query more than a few neighboring blocks. My wild guess is that it gets more efficient somewhere at about 10 blocks queried.

[Lapayo]

I don´t get the point, why I should use the DoWithX functions.
Whats better with them?
I think:

Code:

cEntity *ent = m_Player->GetWorld()->GetEntityById(Id);
if(ent && ent->IsA("cPawn"))
{
    reinterpret_cast<cPawn*>(ent)->TakeDamage(1, m_Player);
}

is much faster to write, needs less knowledge of the system (You don´t need to know all the different Callback classes) and in addition looks much cleaner than those big classes.
The current implementation of the above code:

Code:

    class cDamageEntity : public cEntityCallback
    {
        virtual bool Item(cEntity * a_Entity) override
        {
            if( a_Entity->IsA("cPawn") )
            {
                reinterpret_cast< cPawn* >( a_Entity )->TakeDamage(Damage, Instigator );
            }
            return true;
        }
    public:
        int Damage;
        cEntity * Instigator;
    } Callback;

    Callback.Damage = 1; // TODO: Find proper damage from current item equipped
    Callback.Instigator = m_Player;

    cWorld * World = m_Player->GetWorld();
    World->DoWithEntityByID(a_Packet->m_TargetID, Callback);

[xoft]

One word: multithreading
If you leak plain pointers to Lua, you're asking for trouble - how does Lua know that someone hasn't deleted the object?
And if you use shared pointers, you're enforcing a mutex lock for each access to the pointer, which is slow.
So with the callbacks, you're multithreading-safe and still don't incure that much of a performance penalty.

This could have been really seen with chunks in the early multithreading-enabled revisions (at around Rev 170 ~ 300), where the server would crash randomly because not even a shared_ptr is multithreading-safe.

[Lapayo]

For LUA this might be true, but I´m talking about the C++ code.
My Code executes the Code on the Entity also directly after getting the pointer to it. So there should be no mt problem. (If there is no thread deleting the client if it´s used, but this could also happen if you use the DoWithEntity function)
Or am I missing something?

[xoft]

C++ is the same.
Your code in thread A grabs a pointer. It gets pre-empted by thread B. Thread B deletes the entity. Now thread A has an invalid pointer and is gonna call its method. Bang!

---

The point of DoWithEntity() is that while it's executing (including the callback), the internal lock is held, guaranteeing that no other thread is manipulating (especially deleting) entities.