Revamp Delegates

[Phantom139]

This is being pushed to item 1 on the to-do list once programming gets going again when I finish up my ultimate guide project.

@Ragora has created a very easy to use Delegate system (https://github.com/Ragora/EasyDelegate) that can replace the massive header enforced system that we currently have.

This is just a note to strip out the old system in favor of implementing that one right away.

[Ragora]

I've been doing a lot of adjustments to the system ever since I posed it to you. It's current state I should probably clean up and mark as version 1, though. Would be nice if I can make it cleaner but apparently there's no real good way to separate declaration and implementation when you're using templates.

Should probably get around to writing up decent docs on it as well, Doxygen seems to dislike templates.

Just note that it is reliant upon the compiler supporting variadic templates, and for MSVS they apparently don't support 'true' variadic templates: http://blogs.msdn.com/b/vcblog/archive/2011/09/12/10209291.aspx (see: 'Faux variadics' -- "At that point, we'll see if we can raise the _VARIADIC_MAX default to 10 again (as I would prefer not to break existing code unnecessarily).")

I'm not sure if GCC has a similar limitation.

[Phantom139]

Well, as mentioned, there's still plenty of time. I'm not seeing this guide project being finished until at the soonest, mid to late October, so there's still plenty of time to go there.

I'll have to look into the MSVS issue to see if it can have any level of support needed for at least basic implementation.

[Ragora]

https://github.com/Ragora/EasyDelegate/releases Open an issue on the project page or talk to me if you run into any problems with the library. The current todo list can be found here. The issue tracker just references the TODO list entries.

[Phantom139]

Just a quick update here. It would appear that everything here should port in perfectly fine in terms of C++11 compliance needed.

According the the MSVS info docs, VS2013 already has support for Variadic Templates as noted here: http://blogs.msdn.com/b/vcblog/archive/2013/12/02/c-11-14-core-language-features-in-vs-2013-and-the-nov-2013-ctp.aspx

I've been building the solution thus far on a copy of VS2013, and probably will switch over to VS2014 at some point to keep things "current".

[Ragora]

Seems like it would've worked with up to 10 arguments using faux variadics anyway, more than enough I believe.

Also you should use CMake, it will keep you independent of IDE. The EasyDelegate project has a simple example of how to use it. (And you had to use it anyway to generate the VS13 project, unless you made that manually.)

[Phantom139]

Alright, well I've decided to put a day of work towards G2D to install V1.1 today. I'll let you know if I encounter any problems along the way. Also, since the engine is going to be released as MIT source, would you be willing to change the licensing of the easydelegate to MIT as well (I'd likely be needing to change some of the code for compatibility with the engine in terms of swapping out STL stuff with already made engine classes). If not, I'd be willing to package it as an external lib to use in the engine as well.

[Phantom139]

Alright, so commit dd52d09 has implemented the system into the engine. There is still a little bit of cleanup work to do in the existing code that used the old delegate system, so I'll leave this issue open until those are resolved.

[Ragora]

Yea, I don't mind it being MIT. It probably should have been MIT to begin with, really.

https://github.com/Ragora/EasyDelegate/releases/tag/v1.2-MIT

[Phantom139]

Alright, so a little update here. I went ahead and did a little cleanup session today on the engine files, particularly with the delegate replacement (57209db), and apparently for some interestingly weird reason, the compiler is having some issues comparing the delegate objects here:

void removeTickerInstance(const GalacticFrameTickerDelegate &dele) { for (S32 i = 0; i < tickerList.size(); i++) { if (tickerList[i].tickerDelegate == dele) { tickerList.erase((U32)i); } } }

In this particular case the compiler is complaining that it cannot perform the comparison operator between GalacticFrameTickerDelegate and const GalacticFrameTickerDelegate &, which are typedef'd as typedef StaticDelegate<bool, F64> GalacticFrameTickerDelegate;

More investigation is needed.

[Ragora]

At a quick glance, all I can see is that there is const-ness disparity. I don't actually have a Windows box to build on hand to look at the exact error. Also, you have a spelling error in your include in engineCore.h, line 61: https://github.com/PhantomGamesDevelopment/Galactic-2D/blob/950e740507d6538e1c377b5907a09db5c3c0248a/Source/EngineCore/engineCore.h#L61 Edit: I forgot I had write permissions to the repo, so I went ahead and corrected it.

I noticed that there is no real (easy) way to set up a build environment for the engine. Here is the start of a CMakeLists file if it helps at all (to generate project files): http://pastebin.com/SdAsm4tL I don't know if the thing will actually complete a build as I haven't actually gotten to try it on a Windows box yet, but it should at least succeed during the build stage. The link stage will probably be where it fails.

I'm also going to note that I discovered a bug in the library's Cached Delegate (deferred calls) implementations, if you had intended on using those. Methods with references in their parameters will cause the CachedDelegate to save stack space variables and thusly pass in garbage when the eventual call is made. This is already fixed on my part by forcing the removal of reference types (using std::remove_reference) so that they are cached by actual value, not reference. My coding is just pending a commit.

Beyond that I have been working at optimizations for the overall library. For the CachedDelegate types, I am removing the necessity of having to allocate a secondary delegate object and just storing the delegate properties directly (thusly removing a couple needless potentially vtabled calls).

[Phantom139]

Alright, didn't catch that spelling error earlier so thanks for that.

I'm mainly coding in the virtual function overrides for the platform specific code at the moment which is why I haven't really had the opportunity to go and do a full build of the project as of lately to see where there are issues, but that will be a top priority once I finish up the windows platform functions (#21).

As for the upgrades and fixes to the library itself, you do have write access to the repository, so go ahead and install the updates as you see fit, or just let me know when there's a new one to install and I'll take care of it. Just remember that I am leaning more towards using internal engine classes over the standard template library at the moment to ensure full compatibility across all platforms when that time comes.

[Ragora]

Most of my proposed changes are in the repo right now: https://github.com/Ragora/EasyDelegate/blob/master/include/easydelegate.hpp

I just need to look over it, update the commenting (for Doxygen) and verify that everything works correctly. You should be relatively fine API-wise but there is a couple of internal changes that qualified it the major version increment (as it is an API breaking change). Here is a full summary of the main points:

• I had moved from using std::vector to std::set and std::unordered_set wherever it would better fit. Your DynArray should be able to easily replace these but I had decided that those sets were more optimal choices.
• I had to use std::remove_reference to force storage of non-reference types in the std::tuple used to store call information in all cached delegate types. This was to ensure that no stack-space (temporary) variables were stored in the tuple. If you had a reason to use the cached delegate types, which you probably do, you should be able to implement an std::tuple type yourself using templates. I am unsure as to how std::remove_reference works, though.
• I created my own exception types to better distinguish EasyDelegate errors from those thrown by the methods that they call.
• DelegateSet now literally derives from std::set, but it still operates exactly the same as it did before. You just need to make sure that you never accidentally use a DelegateSet pointer to refer to an instance of std::set, but considering the usage cases here, that shouldn't occur.
• Both CachedMemberDelegate and CachedStaticDelegate now derive from from GenericTypedCachedDelegate which derives from GenericCachedDelegate. The GenericTypedCachedDelegate has the typed dispatch() method available to it to get the return value of the called method but obviously the return type needs to be known ahead of time. The GenericCachedDelegate is as generic as a cached delegate can get and has the typeless generic_dispatch() available to it. That means that all cached delegate types can be stored in a single container as a GenericCachedDelegate regardless of ... everything.
• I had better qualified a handful of methods which should help generate more efficient code. This means: noexcept qualifiers were added to some methods to signal to the compiler that they will never raise an exception. A method here and there were marked to be forced inline if EASYDELEGATE_FORCE_INLINE is defined. Defining this seems to reduce CachedDelegate call complexity by a single call.

These are changes that I still have to decide on:

• I had considered making auto-update delegates where if a delegate were to be destroyed, it would automatically remove itself from any DelegateSet is in. Considering that I have a literal DelegateSet type, I should probably do this.
• I wanted to make an UnorderedDelegateSet type (that uses std::unordered_set instead) but as of thus far, I couldn't find a way to do that without copying a ton of code.

[Phantom139]

I'll likely perform the upgrade once I finish up the work with the Cross Platform code as right now, I'd rather focus on that work-load to get it done and out of the way.

As for the latest updates, 5e4bcf3 finished the code sweep to fully integrate the lib into the code-base so things should be all set to go in terms of using it in any additional cases where delegates will be required.

As for the error located on line 114 (https://github.com/PhantomGamesDevelopment/Galactic-2D/blob/development/Source/EngineCore/Tools/frameTicker.h#L114), simply changing the types to match exactly does not please the compiler, so something else seems to be going on.

The error as caught by MSVS IS states:

IntelliSense: no operator "==" matches these operands operand types are: Galactic::Core::GalacticFrameTickerDelegate == const Galactic::Core::GalacticFrameTickerDelegate c:\Users\Owner\Documents\Phantom Games Development\Galactic 2D\Galactic2D\Galactic2D\Source\EngineCore\Tools\frameTicker.h 114 40 Galactic 2D

Adjusting the types to matches produces the same error. Fixing this up will resolve the final piece as needed by this issue.

[Ragora]

There is no way to compare for equality. If you wanted to simply check if they were the exact same instance then you could try: if (&(tickerList[i].tickerDelegate) == &dele) // Compare the pointers

Otherwise, you will need to compare mProcAddress and so on in an == operator definition. I should probably implement this myself in the lib, actually.

[Phantom139]

Actually, that's exactly what was needed by the code. The bit there is just running through the list of delegates stored on the main ticker and if it's the matching one (in the event of removeTickerInstance) then it deletes it.

That should work fine in that case, thanks. Commit & Close coming up.

[Phantom139]

Final delegate related error associated with the implementation of EasyDelegate has been addressed in 979329b. Closing & marking for future reference.