/cuberite

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Channel Management #2384

Closed
wants to merge 10 commits into
base: master
from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@SamJBarney @madmaxoft @tigerw @sphinxc0re @worktycho @bearbin
@SamJBarney
Contributor

SamJBarney commented Jul 23, 2015

  • Allows plugins to claim a plugin channel and assign a callback, making it the only one that is called.
  • If no plugin owns a channel, then it goes through the hook system like normal.
  • Plugins can assign a client handle to a channel.
  • Plugins can remove a client handle from a channel.
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

I know it is failing right now because a file isn't being generated. I had this fixed once, now I'm trying to figure it out again. Any ideas?
Contributor

SamJBarney commented Jul 23, 2015

I know it is failing right now because a file isn't being generated. I had this fixed once, now I'm trying to figure it out again. Any ideas?
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

The first time this happened I had a mismatched tolua_begin and tolua_end
Contributor

SamJBarney commented Jul 23, 2015

The first time this happened I had a mismatched tolua_begin and tolua_end
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

Figured it out. Code fails to compile with make -j4, but compiles fine with make.
Contributor

SamJBarney commented Jul 23, 2015

Figured it out. Code fails to compile with make -j4, but compiles fine with make.
@madmaxoft

madmaxoft reviewed Jul 23, 2015
View changes

Show outdated Hide outdated src/Bindings/ManualBindings.cpp
@madmaxoft

madmaxoft reviewed Jul 23, 2015
View changes

Show outdated Hide outdated src/ChannelManager.cpp
@madmaxoft

madmaxoft reviewed Jul 23, 2015
View changes

Show outdated Hide outdated src/ChannelManager.cpp
@madmaxoft

madmaxoft reviewed Jul 23, 2015
View changes

Show outdated Hide outdated src/ChannelManager.cpp
@madmaxoft

madmaxoft reviewed Jul 23, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@madmaxoft

madmaxoft reviewed Jul 23, 2015
View changes

Show outdated Hide outdated src/ClientHandle.cpp
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

So what it looks like I am running into here is that by the time a file that requires the generated files is being processed, the files haven't had the time to generate, and so the build fails.
Contributor

SamJBarney commented Jul 23, 2015

So what it looks like I am running into here is that by the time a file that requires the generated files is being processed, the files haven't had the time to generate, and so the build fails.
@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Jul 23, 2015

Member

Which files are they? Perhaps a dependency can be added to cmake
Member

madmaxoft commented Jul 23, 2015

Which files are they? Perhaps a dependency can be added to cmake
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

Well, its the files generated by tolua. https://github.com/cuberite/cuberite/blob/master/src/Bindings/CMakeLists.txt#L50
Contributor

SamJBarney commented Jul 23, 2015

Well, its the files generated by tolua. https://github.com/cuberite/cuberite/blob/master/src/Bindings/CMakeLists.txt#L50
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

@worktycho suggested doing this to all files that require the generated files.
Edit: Isn't helping. *Insert frustrated noises here *
Contributor

SamJBarney commented Jul 23, 2015

@worktycho suggested doing this to all files that require the generated files.
Edit: Isn't helping. *Insert frustrated noises here *
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 23, 2015

Contributor

Yes! Got it!
Contributor

SamJBarney commented Jul 23, 2015

Yes! Got it!
@madmaxoft

madmaxoft reviewed Jul 24, 2015
View changes

Show outdated Hide outdated src/ChannelCallback.cpp
@madmaxoft

madmaxoft reviewed Jul 24, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@madmaxoft

madmaxoft reviewed Jul 24, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@madmaxoft

madmaxoft reviewed Jul 24, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@madmaxoft

madmaxoft reviewed Jul 24, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 26, 2015

Contributor

Something separate from forge that I am working on. I'm just going to remove those from being exported.
Contributor

SamJBarney commented Jul 26, 2015

Something separate from forge that I am working on. I'm just going to remove those from being exported.
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 28, 2015

Contributor

This good for me to squash?
Contributor

SamJBarney commented Jul 28, 2015

This good for me to squash?
@madmaxoft

madmaxoft reviewed Jul 28, 2015
View changes

Show outdated Hide outdated src/ClientHandle.h
@madmaxoft

madmaxoft reviewed Jul 28, 2015
View changes

Show outdated Hide outdated src/ClientHandle.h
@madmaxoft

madmaxoft reviewed Jul 28, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@madmaxoft

madmaxoft reviewed Jul 28, 2015
View changes

Show outdated Hide outdated src/ChannelManager.h
@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Jul 28, 2015

Member

There's still something missing - the Lua API documentation. You've exported a new class and several functions, they need documentation.

Also, since this is a rather big topic, perhaps even an article on how to use plugin channels properly, with examples, would be in order.
Member

madmaxoft commented Jul 28, 2015

There's still something missing - the Lua API documentation. You've exported a new class and several functions, they need documentation.

Also, since this is a rather big topic, perhaps even an article on how to use plugin channels properly, with examples, would be in order.
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 30, 2015

Contributor

Where do I need to put the Lua API documentation?
Also, is there a good tutorial for doxy comments?
Contributor

SamJBarney commented Jul 30, 2015

Where do I need to put the Lua API documentation?
Also, is there a good tutorial for doxy comments?
@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Jul 30, 2015

Member

Basically we use comments like this, nothing fancy:

/** Breaks the server. Returns the amount of screwups made.
a_ShouldExplode specifies whether the computer should explode while breaking the server. */
int BreakServer(bool a_ShouldExplode);

The API documentation is in the APIDump plugin, $/MCServer/Plugins/APIDump/APIDesc.lua; some groups of classes are documented separately in the Classes subfolder and all hooks are documented in the Hooks subfolder. Articles are simple HTML files with other static files, declared at the bottom of the APIDesc.lua file.
Use the api console command to export the API into the $/MCServer/API folder, then apishow console command to open it in the browser. The plugin also dump all unexported symbols into the $/MCServer/API/_undocumented.lua file in a format suitable for direct copying into the APIDesc.lua file.
Member

madmaxoft commented Jul 30, 2015

Basically we use comments like this, nothing fancy:

/** Breaks the server. Returns the amount of screwups made.
a_ShouldExplode specifies whether the computer should explode while breaking the server. */
int BreakServer(bool a_ShouldExplode);

The API documentation is in the APIDump plugin, $/MCServer/Plugins/APIDump/APIDesc.lua; some groups of classes are documented separately in the Classes subfolder and all hooks are documented in the Hooks subfolder. Articles are simple HTML files with other static files, declared at the bottom of the APIDesc.lua file.
Use the api console command to export the API into the $/MCServer/API folder, then apishow console command to open it in the browser. The plugin also dump all unexported symbols into the $/MCServer/API/_undocumented.lua file in a format suitable for direct copying into the APIDesc.lua file.
@tigerw

This comment has been minimized.

Show comment
Hide comment
@tigerw

tigerw Jul 30, 2015

Member

Did...did xoft just use humour?
Member

tigerw commented Jul 30, 2015

Did...did xoft just use humour?
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Jul 30, 2015

Contributor

Hmmmm... must be pretty dry. I can't see it unless it's the fake comment he wrote.
Contributor

SamJBarney commented Jul 30, 2015

Hmmmm... must be pretty dry. I can't see it unless it's the fake comment he wrote.
@tigerw

This comment has been minimized.

Show comment
Hide comment
@tigerw

tigerw Jul 30, 2015

Member

That BreakServer thing. I'm not sure if I've ever seen a code-joke from xoft prior to this.
Member

tigerw commented Jul 30, 2015

That BreakServer thing. I'm not sure if I've ever seen a code-joke from xoft prior to this.

SamJBarney added some commits Aug 4, 2015

@SamJBarney
Plugins may now register a handler for a specific channel instead of … 
…waiting for their HOOK_PLUGIN_MESSAGE callback to be called.

All channel messages get routed through cChannelManager first. If there is not a callback for the specific channel, the message gets passed off to the hook system.
Loading status checks…
71fe6a7
@SamJBarney
Fixed spacing, comment formatting, and spelling.
Loading status checks…
b03aded
@SamJBarney
Fixed stray docy-comment
Loading status checks…
a9720d7
@SamJBarney
Added changes I missed during the merge.
Loading status checks…
37b9860
src/ChannelCallback.cpp
@@ -0,0 +1,21 @@
#include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 4, 2015

Member

Usually we put a header comment into the file saying what it is and what it does, high-level. So:

// ChannelCallback.cpp

// Implements the cChannelCallback class representing the callbacks used for plugin channel management
Copy link
@madmaxoft

madmaxoft Aug 4, 2015

Member

Usually we put a header comment into the file saying what it is and what it does, high-level. So:

// ChannelCallback.cpp

// Implements the cChannelCallback class representing the callbacks used for plugin channel management
src/ChannelCallback.h
{
int m_FnRef;
public:
cChannelCallback(cPluginLua & a_Plugin, int a_FnRef) : cPluginLua::cResettable(a_Plugin), m_FnRef(a_FnRef){}

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 4, 2015

Member

Split into separate lines
Copy link
@madmaxoft

madmaxoft Aug 4, 2015

Member

Split into separate lines

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Aug 4, 2015

Contributor

The constructor or inheritance? I'm not sure which one you are requesting.
Copy link
@SamJBarney

SamJBarney Aug 4, 2015

Contributor

The constructor or inheritance? I'm not sure which one you are requesting.

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 4, 2015

Member

Both.

cClass(cParam & a_Param):
  m_Member(a_Param)
{
}
Copy link
@madmaxoft

madmaxoft Aug 4, 2015

Member

Both.

cClass(cParam & a_Param):
  m_Member(a_Param)
{
}
@madmaxoft

madmaxoft reviewed Aug 4, 2015
View changes

Show outdated Hide outdated src/Protocol/Protocol.h
@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 4, 2015

Member

The code could overall use a bit more comments, especially for the new classes.
Member

madmaxoft commented Aug 4, 2015

The code could overall use a bit more comments, especially for the new classes.

SamJBarney added some commits Aug 4, 2015

@SamJBarney
Added header comments.
Loading status checks…
799661a
@SamJBarney
Added doxy-comment
Loading status checks…
70667dc
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Aug 4, 2015

Contributor

The comments will be put in place, along with the necessary documentation.
Contributor

SamJBarney commented Aug 4, 2015

The comments will be put in place, along with the necessary documentation.
@sphinxc0re

This comment has been minimized.

Show comment
Hide comment
@sphinxc0re

sphinxc0re Nov 5, 2015

Contributor

Any update on this one?
Contributor

sphinxc0re commented Nov 5, 2015

Any update on this one?
src/Bindings/ManualBindings.cpp
self->RegisterChannel(Channel, Callback);
// Cut away everything from the stack except for the cChannelManager reference; return that:
lua_settop(L, 1);

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Nov 6, 2015

Member

I'm not sure if this works properly for nested (Lua-C++-Lua-C++) calls.
Copy link
@madmaxoft

madmaxoft Nov 6, 2015

Member

I'm not sure if this works properly for nested (Lua-C++-Lua-C++) calls.

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Nov 7, 2015

Contributor

@madmaxoft What do you mean exactly?
Copy link
@SamJBarney

SamJBarney Nov 7, 2015

Contributor

@madmaxoft What do you mean exactly?

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Nov 9, 2015

Member

No, ignore my comment, I'm getting rusty in my C++-Lua bindings knowledge. Sorry.
Copy link
@madmaxoft

madmaxoft Nov 9, 2015

Member

No, ignore my comment, I'm getting rusty in my C++-Lua bindings knowledge. Sorry.
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Nov 7, 2015

Contributor

Sorry @sphinxc0re, I've been busy with school. I'll see if I can get some time to work on it next week.
Contributor

SamJBarney commented Nov 7, 2015

Sorry @sphinxc0re, I've been busy with school. I'll see if I can get some time to work on it next week.
src/Bindings/ManualBindings.cpp
if (!L.CheckParamString(2) || !L.CheckParamFunction(3))
{
return 0;
}

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Nov 9, 2015

Member

Usually we group all these checks (CheckParam...) together at the function's start.
Copy link
@madmaxoft

madmaxoft Nov 9, 2015

Member

Usually we group all these checks (CheckParam...) together at the function's start.
src/ChannelManager.cpp
}
}
void cChannelManager::HandlePluginUnloading(const cPluginLua * a_Plugin)

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Nov 9, 2015

Member

Style: 5 empty lines between function bodies
Copy link
@madmaxoft

madmaxoft Nov 9, 2015

Member

Style: 5 empty lines between function bodies

bearbin and others added some commits Nov 24, 2015

@bearbin
Merge pull request #2688 from bendl/master 
Fixed 'Install on DigitalOcean' link in README.md
Loading status checks…
ae850e7
@SamJBarney
Merge branch 'ChannelManagement' of https://github.com/SamJBarney/cub… 
…erite into ChannelManagement

Conflicts:
	Server/Plugins/ChatLog
	Server/Plugins/ChunkWorx
	Server/Plugins/Handy
	Server/Plugins/MagicCarpet
	Server/Plugins/TransAPI
	lib/cmake-coverage
	lib/expat
	lib/lua
	lib/luaexpat
	lib/luaproxy
	lib/sqlite
	lib/tolua++
	lib/zlib
	src/ClientHandle.h
763b597
@SamJBarney
Added missing forward definitions.
Loading status checks…
eb511e4
@SamJBarney
Fixed spacing issue.
Loading status checks…
faa7a16
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Nov 24, 2015

Contributor

I could have swore I merged in the changes from master to my branch correctly...
Contributor

SamJBarney commented Nov 24, 2015

I could have swore I merged in the changes from master to my branch correctly...
@SamJBarney

This comment has been minimized.

Show comment
Hide comment
@SamJBarney

SamJBarney Nov 24, 2015

Contributor

grumble Gotta track down my comments that disappeared.
Contributor

SamJBarney commented Nov 24, 2015

grumble Gotta track down my comments that disappeared.

@sphinxc0re sphinxc0re closed this Mar 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment