search

fungos / cr

cr.h: A Simple C Hot Reload Header-only Library
https://fungos.github.io/cr-simple-c-hot-reload/
MIT License
1.57k stars 103 forks source link

OSX: fails to find imageFile #16

Closed septag closed 5 years ago

septag commented 5 years ago

OSX implementation: in cr_plugin_validate_sections function. right here inside the loop:

        if (strcasecmp(name, imagefile.c_str())) {
            // match loaded image filename
            continue;
        }

It doesn't find a match if I submit the relative path to cr_plugin_load, because the imageFile is absolute path. I can submit a pull request, but I'm not sure is this a real bug or I'm not complying with the API or something.. does the API assume that the plugin path is always absolute ?

fungos commented 5 years ago

No, we do not assume it should be absolute. I would welcome a PR fixing this. Thank you for trying cr!

septag commented 5 years ago

Thank you for this great lib. Actually I've been playing with it for some time and submitted another issue before.

I have some more suggestions to improve the lib and will be glad to submit a pull request if you are ok with them:

1) overriding some stuff like memory allocations and asserts using macros. unfortunately some of allocations are inside std::string and stl stuff. but the C-style ones can easily be overridden 2) Add a macro to override the cr_main symbol 3) Port the whole thing to C. this a a big change and I'm not sure you really like it. But I'm using it in my personal C project and I can put some time into it. It makes CR to be used inside C units and also removes the stl dependency. 4) In my own fork, I'm also keeping another symbol (callback) to broadcast events to plugins. we can put this into the cr_load_plugin API and it can be optional. The host can call a function like cr_call_func to call the function in plugins.

fungos commented 5 years ago

I'm not opposed to make it C pure and I may also accept a well tested PR for said conversion. I agree with 1 and 3, the problem for me right now is time and osx testing. About 2 I'm not against it too, but it need to be sure it is made in a safe and backwards compatible way, it is not that hard anyway. 4 I would like a bit more explanation on the use case and the API, if you can expand over this it would be great.

In any case, the most important thing is to be sure we keep it stable, safe and backwards compatible. :)

If you're willing to do this, please make new issues for each of these and then make it small modifications at a time so we can review and merge safely.

Thank you a lot for the feedback and for the PR!