Critical bug in MCServer - Printable Version

Critical bug in MCServer - Printable Version

+- Cuberite Forum (https://forum.cuberite.org)
+-- Forum: Cuberite (https://forum.cuberite.org/forum-4.html)
+--- Forum: Development (https://forum.cuberite.org/forum-13.html)
+--- Thread: Critical bug in MCServer (/thread-254.html)

Pages: 1 2 3 4

RE: Critical bug in MCServer - FakeTruth - 02-15-2012

(02-15-2012, 12:24 AM)xoft Wrote: Callbacks are a great way of doing things, though I'd prefer class-callbacks (see cWorld::ForEachPlayer() for an example). Instead of passing a function pointer, you pass a pointer to a class that has overridden a specific virtual method that does the actual work. The advantage is that the class can carry more data (parameters, accumulators etc), the disadvantage is that probably this won't work with Lua (I have no idea).

It _can_ work with Lua, unforunately even though Lua can work with objects, Lua itself is not really object orientated and has some extremely basic object inheritance.
To inherit objects you need to write some really ugly code like this:

Code:
local ChatLogPlugin = {}

ChatLogPlugin.__index = ChatLogPlugin

function ChatLogPlugin:new()

   local t = {}

   setmetatable(t, ChatLogPlugin)

   local w = Lua__cPlugin:new()

   tolua.setpeer(w, t)

   w:tolua__set_instance(w)

   return w

end

Here ChatLogPlugin practically inherits cPlugin by having Lua__cPlugin forward calls to the Lua object. But I don't like it because it requires this piece of code, it's confusing and ugly.

I think we simply need special implementations of those ForEach* functions to work with Lua function callbacks. I don't think the carrying more data is an issue in Lua, because you can simply store that data externally somewhere, or supply an additional Lua table to the function.

Code:
local SayHiData = 0

function Initialize( Plugin )

    cRoot:Get():GetWorld():ForEachPlayer( SayHi, SayHiData )

    return true

end

function SayHi( Player, Data )

    SayHiData = SayHiData + 1

    Player:SendMessage("Hi player ".. SayHiData .. "!")

end

In this example it sends a number along, but it could also send a table reference with much more data. So you could say that SayHiData is the class instance, and it could even be made an optional parameter.

RE: Critical bug in MCServer - xoft - 02-15-2012

Special implementations sound ok, as long as there aren't too many. I wouldn't have wanted to call something like:

Code:
m_World->DoInChunk(ChunkX, ChunkZ, cSetBlock(BlockX, BlockY, BlockZ, E_BLOCKID_IRONORE));

Instead, we'll keep the SetBlock() function, calling this instead:

Code:
m_world->SetBlock(BlockX, BlockY, BlockZ, E_BLOCKID_IRONORE);

However, those callback classes are still useful - see the webplugin's cWebAdmin.cpp, class cPlayerAccum. It includes a string buffer into which all the players are accummulated, HTML-formatted, one-by-one, in a single function call. Thread-safe and everything.
The thread safety is of utmost importance here - the ForEachXXX() function needs to lock the list of objects it is working on, process the callbacks and unlock the list - so that no other thread can manipulate the list under its hands.
Calling Lock, GetAll, Process, Unlock is not a good solution because someone, somewhere is always gonna forget to lock or unlock (and RAII doesn't work well for this)

Finally, I have a vision that the future cChunkMap object will not let a single cChunkPtr out of it, it will only accept coordinates and actions as the parameters for all its functions. Thus, it could finally be pronounced thread-safe, including chunk access Smile

I think the current "bad compressed data format" errors are the result of two threads colliding on a single chunk - one thread writes blockdata, the other thread is in the compressino routine. The compression looks at a value, decides to use some algorithm on it and before the algorithm starts, the value gets changed by the other thread -> bang, invalid compressed data. I'll get working on this issue as soon as possible.

RE: Critical bug in MCServer - NiLSPACE - 02-15-2012

maybe when the new map format comes is it easier to fix this bug

RE: Critical bug in MCServer - FakeTruth - 02-15-2012

So I couldn't wait and implemented the cWorld:ForEachPlayer() function in Lua http://code.google.com/p/mc-server/source/detail?r=260

I couldn't figure out how to pass a Lua table to C++ and then back to Lua though Sad

I need some help with this. It's kinda annoying to use when you can't pass some arbitrary values to the callback

RE: Critical bug in MCServer - xoft - 02-15-2012

Actually I was thinking that the Lua interface objects needn't 100% correspond to MCS internal objects.

I was thinking, how about a (C++) wrapper for let's say the cPlayer, that only stores a pointer to the world (safe - noone deletes the world while the server is running) and the player name. Then every time Lua script attempted an operation on this player, let's say "kick", the Lua interface object would call something like "m_World->KickPlayer(m_PlayerName)". Same with entities - store world pointer and entity ID. Let the world object enumerate all cEntities safely and return an array of "cLuaEntity" classes.

This way we can sustain the current Lua interface with the objects the way there are, with direct functions and even "get all" functions, but in the inside work with the safe interfaces. It would mean a lot of coding, though, and I'm afraid a lot of internal Lua learning, especially about object lifecycle - when to delete objects that were passed out to Lua

(02-15-2012, 05:16 AM)FakeTruth Wrote: It's kinda annoying to use when you can't pass some arbitrary values to the callback

I've seen the code now and I don't know - why can't you? I think the Lua function ForEachPlayer could perfectly take 2 parameters - the function to call and an arbitrary pointer. Store the pointer in the callback class and then call the Lua callback function with that pointer on the stack.
Unfortunately I can't help you implement this, as I have no experience in Lua integration, it's just what I've seen in the code makes me believe that it's possible to do.

RE: Critical bug in MCServer - FakeTruth - 02-15-2012

(02-15-2012, 06:36 AM)xoft Wrote: Actually I was thinking that the Lua interface objects needn't 100% correspond to MCS internal objects.

I was thinking, how about a (C++) wrapper for let's say the cPlayer, that only stores a pointer to the world (safe - noone deletes the world while the server is running) and the player name. Then every time Lua script attempted an operation on this player, let's say "kick", the Lua interface object would call something like "m_World->KickPlayer(m_PlayerName)". Same with entities - store world pointer and entity ID. Let the world object enumerate all cEntities safely and return an array of "cLuaEntity" classes.

This way we can sustain the current Lua interface with the objects the way there are, with direct functions and even "get all" functions, but in the inside work with the safe interfaces. It would mean a lot of coding, though, and I'm afraid a lot of internal Lua learning, especially about object lifecycle - when to delete objects that were passed out to Lua

That sounds like an aweful lot of work though, and it would probably be slow? If you want to do something to all entities on the server you request a list of entity ID's after which all function you call on them looks up the actual entity by ID and calls the function. This does not sound very efficient...

(02-15-2012, 06:36 AM)xoft Wrote:
(02-15-2012, 05:16 AM)FakeTruth Wrote: It's kinda annoying to use when you can't pass some arbitrary values to the callback
I've seen the code now and I don't know - why can't you? I think the Lua function ForEachPlayer could perfectly take 2 parameters - the function to call and an arbitrary pointer. Store the pointer in the callback class and then call the Lua callback function with that pointer on the stack.
Unfortunately I can't help you implement this, as I have no experience in Lua integration, it's just what I've seen in the code makes me believe that it's possible to do.

I can't because I don't know how Tongue

I know it's possible!

RE: Critical bug in MCServer - xoft - 02-15-2012

(02-15-2012, 07:10 AM)FakeTruth Wrote: That sounds like an aweful lot of work though, and it would probably be slow? If you want to do something to all entities on the server you request a list of entity ID's after which all function you call on them looks up the actual entity by ID and calls the function. This does not sound very efficient...

That's not exactly what i meant. This interface would be used only if you wanted to do something with all the entities (or most of them at least); you could still use GetEntityByID() if you know the ID

(02-15-2012, 07:10 AM)FakeTruth Wrote: I can't because I don't know how I know it's possible!

That's a surprise for me, considering the Lua glue code you've written so far, i'd say that adding a single parameter to a function should be a piece of cake for you Wink

RE: Critical bug in MCServer - FakeTruth - 02-15-2012

(02-15-2012, 07:14 AM)xoft Wrote:
(02-15-2012, 07:10 AM)FakeTruth Wrote: That sounds like an aweful lot of work though, and it would probably be slow? If you want to do something to all entities on the server you request a list of entity ID's after which all function you call on them looks up the actual entity by ID and calls the function. This does not sound very efficient...
That's not exactly what i meant. This interface would be used only if you wanted to do something with all the entities (or most of them at least); you could still use GetEntityByID() if you know the ID

I don't get it, haha

(02-15-2012, 07:14 AM)xoft Wrote:
(02-15-2012, 07:10 AM)FakeTruth Wrote: I can't because I don't know how I know it's possible!
That's a surprise for me, considering the Lua glue code you've written so far, i'd say that adding a single parameter to a function should be a piece of cake for you

I can get a value I expect from Lua in C++ (integer, string, etc.) and then call a Lua function from the C++ side and pass that value in one call. But I don't know how to do it with arbitrary values. I simply want to keep the values in the Lua stack without even reading it in C++ and pass it on to the next function. As you see it goes like this: Lua -> C++ -> Lua. I shouldn't need to access the arbitrary value in C++ and simply pass it on, but I don't know how... Sad

EDIT: I just posted something on the Lua forums http://forum.luahub.com/index.php?topic=3472.0

RE: Critical bug in MCServer - FakeTruth - 02-15-2012

I figured it out yay!
http://forum.luahub.com/index.php?topic=3472.msg8148#new

Ironically I'm not even using this feature.. since you can create functions inside other functions and assign them to variables in Lua Tongue

See what the /playerlist command looks like now:
http://code.google.com/p/mc-server/source/browse/trunk/Plugins/Core/playerlist.lua?spec=svn262&r=262

RE: Critical bug in MCServer - Boo - 02-16-2012

/usr/bin/g++ -s -ggdb -c -I. -Isource -Isource/md5 -IWebServer -Isource/packets -Itolua++-1.0.93/src/lib -Ilua-5.1.4/src -Izlib-1.2.5 -IiniFile -Itolua++-1.0.93/include -Ijsoncpp-src-0.5.0/include -Ijsoncpp-src-0.5.0/src/lib_json source/cPassiveAggressiveMonster.cpp -o build/debug/source/cPassiveAggressiveMonster.o
/usr/bin/g++ -s -ggdb -c -I. -Isource -Isource/md5 -IWebServer -Isource/packets -Itolua++-1.0.93/src/lib -Ilua-5.1.4/src -Izlib-1.2.5 -IiniFile -Itolua++-1.0.93/include -Ijsoncpp-src-0.5.0/include -Ijsoncpp-src-0.5.0/src/lib_json source/cServer.cpp -o build/debug/source/cServer.o
source/cServer.cpp: В функции-члене «virtual bool cServer::ServerCommand(const char*)::cPlayerLogger::Item(cPlayer*)»:
source/cServer.cpp:462:106: ошибка: cannot pass objects of non-trivially-copyable type «const AString {aka const struct std::basic_string<char>}» through «...»
make: *** [build/debug/source/cServer.o] Ошибка 1

help me!!!