Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

New log manager since rev. 4606

<< < (4/9) > >>

thomas:
I'm afraid that I probably won't be able to fix this. I'm unable to follow the modifications to the original code and the reasoning behind them. Not only are there a lot of them, but since half of the code was moved to another file, I can't use diff or blame to see which lines were modified either, so that makes it tedious.

Some of the differences I found are:
SetupGUILogging() is now called before the plugins are loaded. Logger controls are no longer maintained by the application, but are created by hand, either by sending messages or even worse by calling CreateControl manually (see TODO plugin). The behaviour of that function has been changed too (don't know if that is a problem, but it's different).

Trying to find out where the debug log has gone, I went stepping in the debugger. CreateControl() on the Logger in slot 3 (debug log) returns a null pointer, which causes the application to (correctly) ignore it. I don't know why this happens, but apparently somewhere during startup, the TextContolLog is replaced with a NullLog?

mandrav:

--- Quote from: thomas on November 12, 2007, 04:33:08 pm ---I'm afraid that I probably won't be able to fix this. I'm unable to follow the modifications to the original code and the reasoning behind them. Not only are there a lot of them, but since half of the code was moved to another file, I can't use diff or blame to see which lines were modified either, so that makes it tedious.

--- End quote ---

Code has not been changed.
And the move to a different file was done to avoid recompilation of the entire repository each time we want to add a comma or a space.


--- Quote from: thomas on November 12, 2007, 04:33:08 pm ---Some of the differences I found are:
SetupGUILogging() is now called before the plugins are loaded. Logger controls are no longer maintained by the application, but are created by hand, either by sending messages or even worse by calling CreateControl manually (see TODO plugin). The behaviour of that function has been changed too (don't know if that is a problem, but it's different).

--- End quote ---

SetupGUILogging moved there because in the place it was in the original code, no startup log was retained (logs were empty).
I don't understand "controls are not maintained by application". They are. But SDK messages were added so plugins can add/remove loggers at run time (in contrary to only being able to do this at init time).
Also, first step was to make everything compile properly. Old code will slowly be replaced by newer more compact code in time. So the single hack you noticed in todo list is not worth the mention.


--- Quote from: thomas on November 12, 2007, 04:33:08 pm ---Trying to find out where the debug log has gone, I went stepping in the debugger. CreateControl() on the Logger in slot 3 (debug log) returns a null pointer, which causes the application to (correctly) ignore it. I don't know why this happens, but apparently somewhere during startup, the TextContolLog is replaced with a NullLog?

--- End quote ---

I found out why, although I don't understand it.
Seems like Manager::Get()->GetLogManager() and LogManager::Get() return two different instances...
Specifically, in Mgr<MgrT>::Get(), a new instance is created the first time the two above calls are made (totalling 2 instances).

thomas:

--- Quote from: mandrav on November 12, 2007, 06:51:53 pm ---Code has not been changed.
And the move to a different file was done to avoid recompilation of the entire repository each time we want to add a comma or a space.
--- End quote ---
Code has been changed. I found an if(!control) and was sure that I had not put it there. Doing a manual compare with a revision a week old indeed showed that it wasn't there. Now, I don't know if it really matters, but it changes the behaviour of the function from "always create" to "create only if not exist". The reason why I had no such check in the original code was that it should remove one constraint (pay attention to not create duplicate objects) from people who contribute Loggers, and move that constraint into the application. Very likely it works the same now, but the behaviour is different.
I am aware why you moved the code from a header to a source file, and that is perfectly ok... but... since you did this together with some other changes, one cannot see what those changes were, other than by comparing line by line manually. svn diff doesn't show anything useful in that case.


--- Quote ---I don't understand "controls are not maintained by application". They are. But SDK messages were added so plugins can add/remove loggers at run time (in contrary to only being able to do this at init time).
--- End quote ---
What I was saying was that now a dozen different components decide when they want to add a log, instead of this being decided by the application. There is no harm if this is only done by the application in my opinion.
The worst thing to happen is that a user won't have a log after manually loading a plugin until he starts the application for the next time (it will still work in every other respect). That "missing" log is a very minor issue (actually none at all) but it removes a huge deal of complexity and possible errors from the whole system, and makes it a lot easier to follow (for me, at least). But... never mind that. :)


--- Quote ---Seems like Manager::Get()->GetLogManager() and LogManager::Get() return two different instances...
Specifically, in Mgr<MgrT>::Get(), a new instance is created the first time the two above calls are made
--- End quote ---
Oh, it's worse... it looks like that's the case with all managers, only so far nobody ever noticed :(
I don't know why it happens either, it should not be possible, but it is apparently the case.

thomas:
This is an interesting problem.

We have
type* f() { return g(); };
and we have
f()  != g().

This is beyond my understanding of C++ (or mathematics, or anything). I don't see g() return anything but the exact same thing every time.

mandrav:

--- Quote from: thomas on November 13, 2007, 10:40:37 am ---This is an interesting problem.

We have
type* f() { return g(); };
and we have
f()  != g().

This is beyond my understanding of C++ (or mathematics, or anything). I don't see g() return anything but the exact same thing every time.

--- End quote ---

I know why, because it happened to me before (in a different project).
You see, you instantiate the template's "instance" variable in the template header. But this means that different compilation units will create different static vars (because templates are instantiated in the calling unit).

The solution is to remove the static vars instantiation from manager.h and move them in each manager's compilation unit (as template specializations):


--- Code: ("from Manager.h") ---// remove those:

template<class MgrT>MgrT* Mgr<MgrT>::instance = 0;
template<class MgrT>bool  Mgr<MgrT>::isShutdown = false;

--- End code ---


--- Code: ("for example, top of logmanager.cpp") ---template<> LogManager* Mgr<LogManager>::instance = 0;
template<>bool Mgr<LogManager>::isShutdown = false;

--- End code ---

Doing this will make everything work as expected. You can try it, if you want :).

Navigation

[0] Message Index

[#] Next page

[*] Previous page