05-26-2010, 08:44 PM
Quote:Sweet, I'll test them tonight.
Do not hesitate to report any issue or suggestion !
May be it could be interesting to open a new thread for debian packaging stuff (do not know the good place, developper ?).
Pages: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35
05-26-2010, 08:44 PM
05-28-2010, 05:51 PM
Hi Arcum,
Well It takes less time than I expected. I extended the current plugin API with a SetLogFolder function. It basically work as setSettingsDir function. Note I also add a fallback function so it does not break compatibility (I hope). There are still 2 open questions.
1/ Who must handle the following case: calling the callback without changing the directory ? Caller or plugins ? LIke setting I add some test to a m_LogFolder variable to ensure the caller does not recall the callback function when dir is not updated. But in my opinion it is better to let plugins do the job.
2/ confined exceptions to plugins. I read common/include/PluginCallbacks.h (line 63). If I understand correctly std::string (it is equivalent to 'string' ?) must be contained within a try block. But I do not know how to properly code it. An example will be welcome.
********* Patches descriptions (based agains rev 3087)
+ 45_add_zeropad_log_api.patch: Example of the new function implementation in a plugins. Usefull to do some test.
+ 46_add_main_log_api.patch: Core implementation in the emulator of the new function.
PluginManager:
endLogFolder -> call plugins callback to send the directory.
PluginManager:etLogFolder -> Check if folder have changed, and call SendLogFolder
+ 47_use_log_api.patch: Call the new API function. I create a new function GetLogFolder to get the current user Log dir configuration. Then it call SetLogFolder into both Init and Open phases.
Note I previously also added into the Load phase (like SetSettingsFolder) but plugins are not yet loaded. So emulator thinks it already send the dir to the plugins, but they never received it... And so emulator does not resend it... (see point 1 above).
Let's call the code a proof of concept. Any suggestion or issue are welcome.
Well It takes less time than I expected. I extended the current plugin API with a SetLogFolder function. It basically work as setSettingsDir function. Note I also add a fallback function so it does not break compatibility (I hope). There are still 2 open questions.
1/ Who must handle the following case: calling the callback without changing the directory ? Caller or plugins ? LIke setting I add some test to a m_LogFolder variable to ensure the caller does not recall the callback function when dir is not updated. But in my opinion it is better to let plugins do the job.
2/ confined exceptions to plugins. I read common/include/PluginCallbacks.h (line 63). If I understand correctly std::string (it is equivalent to 'string' ?) must be contained within a try block. But I do not know how to properly code it. An example will be welcome.
********* Patches descriptions (based agains rev 3087)
+ 45_add_zeropad_log_api.patch: Example of the new function implementation in a plugins. Usefull to do some test.
+ 46_add_main_log_api.patch: Core implementation in the emulator of the new function.
PluginManager:
endLogFolder -> call plugins callback to send the directory.PluginManager:
etLogFolder -> Check if folder have changed, and call SendLogFolder+ 47_use_log_api.patch: Call the new API function. I create a new function GetLogFolder to get the current user Log dir configuration. Then it call SetLogFolder into both Init and Open phases.
Note I previously also added into the Load phase (like SetSettingsFolder) but plugins are not yet loaded. So emulator thinks it already send the dir to the plugins, but they never received it... And so emulator does not resend it... (see point 1 above).
Let's call the code a proof of concept. Any suggestion or issue are welcome.
05-29-2010, 05:04 AM
(05-28-2010, 05:51 PM)gregory Wrote: [ -> ]1/ Who must handle the following case: calling the callback without changing the directory ? Caller or plugins ? LIke setting I add some test to a m_LogFolder variable to ensure the caller does not recall the callback function when dir is not updated. But in my opinion it is better to let plugins do the job.
I would prefer both Settings and Log folder APIs work the same way, for consistency. There really shouldn't be any issues
(05-28-2010, 05:51 PM)gregory Wrote: [ -> ]2/ confined exceptions to plugins. I read common/include/PluginCallbacks.h (line 63). If I understand correctly std::string (it is equivalent to 'string' ?) must be contained within a try block. But I do not know how to properly code it. An example will be welcome.
In practice it doesn't really matter. The only thing the STL containers will throw exceptions for are new/alloc failures, and really if those fail the app is screwed anyway and is doomed to crash/fail regardless. This is especially true in a multithreaded gui environment -- can't even transport out-of-memory messages to the main thread since cross-thread transport of any sort of useful information requires a series of dynamic allocations anyway.
If you're allocating LARGE chunks of memory, where recoverable out-of-memory exceptions are possible, then yeah you'll want to make sure and trap/handle them. But for basic uses of std::string called primarily during app startup? It would be 'correct' to handle them, but not really useful.
I'll include an example anyway, for sake of learning. There's no need to clutter the code with it, though:
Code:
void CALLBACK PADfail( const char* woot )
{
try {
std:string s( woot );
s += "text!"
m_SaveItForLater = s;
}
catch( std::bad_alloc& ex )
{
// m_SaveItForLater is probably invalid/unallocated, so its not
// like this is a recoverable error. Probably should assert.
assert(false);
}
catch( std::exception& ex )
{
// ..pandemic catch-all clause..
// Some other non-alloc error. Typically these aren't thrown by any
// STL implementations, unless explicitly enabled via compiler define.
// ... probably should just assert, since whatever exceptioned is
// a critical logic error.
assert(false);
}
}Note that I don't use catch(...) -- it has differing behavior cross-compilers, and usually isn't very helpful since nothing in STL throws anything that doesn't derive from std::exception.
(05-28-2010, 05:51 PM)gregory Wrote: [ -> ]Note I previously also added into the Load phase (like SetSettingsFolder) but plugins are not yet loaded. So emulator thinks it already send the dir to the plugins, but they never received it... And so emulator does not resend it... (see point 1 above).
It really should work fine from the Load phase. The reason it didn't for you is because there's also a call to SendSettingsFolder() in the PluginManager's parent class. I'll add a matching SendLogFolder() call there, and that should have it all working fine.
(I'll apply the patch shortly -- really this is needed for both Windows and Linux)
05-29-2010, 05:39 AM
Most of the patch applied. I didn't apply the part pertaining to zeropad, since I can't test it properly, etc. from windows.
@Arcum : I'll handle updating SPU2-X to use the log dir. Its logging code is a bit of a mess, and I need to get rid of the archaic Unicode converter I hacked in years ago (prior to wx), among other things.
All other plugins that have logging (GSdx for example doesn't log anything) can be implemented same as gregory's patch, and same as for the SetSettingsDir. If a plugin doesn't have logging, then there's no need to add the function.
@Arcum : I'll handle updating SPU2-X to use the log dir. Its logging code is a bit of a mess, and I need to get rid of the archaic Unicode converter I hacked in years ago (prior to wx), among other things.
All other plugins that have logging (GSdx for example doesn't log anything) can be implemented same as gregory's patch, and same as for the SetSettingsDir. If a plugin doesn't have logging, then there's no need to add the function.
05-29-2010, 07:06 AM
I tested the zeropad one, and have applied it, after fixing it. (I found a file named "logpadLog.txt" in the pcsx2 directory )
And I'll leave spu2-x up to you.
Btw, I've been wondering about the code in Decoder.cpp in spu2-x. Are there any particular games that use it? I tried commenting out all calls to functions in it once, and didn't particularly notice the difference...
)And I'll leave spu2-x up to you.
Btw, I've been wondering about the code in Decoder.cpp in spu2-x. Are there any particular games that use it? I tried commenting out all calls to functions in it once, and didn't particularly notice the difference...
05-29-2010, 09:42 AM
05-29-2010, 10:17 AM
05-29-2010, 08:24 PM
05-30-2010, 12:53 PM
05-30-2010, 04:45 PM
Arcum,
There was some renaming in the middle of my previous patches by Air. SetLogFolder was renamed setLogDir (to follow setSetttingsDir I think) but not everywhere. Actually it is a real mess At least the callback name must be rename in the main side. (maybe also the callback for consistency)
./pcsx2/PluginManager.cpp:295: { "SetLogFolder", (vMeth*)fallback_SetLogFolder },
Moreover you use the pad callback (PADSetLogFolder which must be renamed PADsetLogDir) in USBnull, FWnull, dev9null.
Enjoy.
Edit: I attached a quick patch to fix the stuff.
Edit: there is another issue. line 235 of ./pcsx2/gui/AppCorePlugins.cpp It use the settings dir instead of the log one. Warning it is not fix in the attached patch
There was some renaming in the middle of my previous patches by Air. SetLogFolder was renamed setLogDir (to follow setSetttingsDir I think) but not everywhere. Actually it is a real mess
At least the callback name must be rename in the main side. (maybe also the callback for consistency)./pcsx2/PluginManager.cpp:295: { "SetLogFolder", (vMeth*)fallback_SetLogFolder },
Moreover you use the pad callback (PADSetLogFolder which must be renamed PADsetLogDir) in USBnull, FWnull, dev9null.
Enjoy.
Edit: I attached a quick patch to fix the stuff.
Edit: there is another issue. line 235 of ./pcsx2/gui/AppCorePlugins.cpp It use the settings dir instead of the log one. Warning it is not fix in the attached patch