cr_main isn't invoked with operation CR_LOAD and the context passed to cr_plugin_load about cr

[skypjack]

I looked at your code.

Actually, it seems that the first call to the plugin takes place when I invoke cr_plugin_update.
In this case, cr_plugin_reload is invoked with the second value I attached to the context because reloadCheck is defaulted to true.
This confirms that I cannot really pass a different value by means of userdata to cr_plugin_load and the one I attached is just ignored.

if I can give you an advice, I would make this thing explicit in the documentation because, at a first reading, I definitely missed it and thought I could use different values for userdata in the different phases.
I apologize for the noise. I close the issue myself having already given an answer. 👍

[fungos]

Oh I see, that was self-inflicted as a protection against typo due the way the docs recommend using the ehdr. I'll try to clean this up better.
(I should set a proper compiler compatibility tests on CI at some time.)

Fixed.

[skypjack]

The fact that both rely on the word load is a bit misleading, I agree.
Fortunately your code is well written and pretty simple to follow and understand, so not a big problem at the end of the day. 👍

May I ask you another thing?
If I got it right, every time I invoke update, it checks if there exists a more updated version of the plugin and eventually the latter is reloaded (unless I pass false to force reload ofc), am I wrong?

[fungos]

Yes, that is exact. That flag was introduced so you can control the frequency of the check and reduce performance impact. It was introduced here #13 by @datgame :)

[skypjack]

Don't worry. You've nothing to apologize for!!
I see it's late for changing the names of these steps and I agree on this. Honestly, I think it would be enough to make it clear in the documentation.
The hot-reload context is just one of the use cases for your library after all and even in this case I could expect a CR_UNLOAD on close because... well, I'm also unloading the plugin. :)

Enjoy your vacancies and sorry for the noise. 👍
Thank you for your response, really appreciated.

[skypjack]

Yeah, sure, I'm using it actually.
It took me just a few minutes to realize that CR_UNLOAD doesn't arrive in this case.
As I said, the code of this library is easy to follow and understand. 👍

[fungos]

Thank you for all your help in here, if there is anything more don't worry reopening this or creating new issues.

[fungos]

Yes, you're right it is confusing. Basically CR_LOAD has nothing to do with cr_plugin_load (which is basically initializer, which was originally named cr_plugin_init). It is a bit confusing and I'll try to improve the documentation soon, thanks for reporting anyway. This issue may still help others.

[skypjack]

I see, good catch actually.
Put aside the dev loop when I iterate continuously, it makes sense to inhibit the test to reload a module in production most of the times.

Thank you for the quick responses. Really appreciated. 👍

[skypjack]

Oh, I've hit another confusing point due to this line:

r = cr_plugin_main(ctx, close ? CR_CLOSE : CR_UNLOAD);

Long story short, when I close a plugin I never see CR_UNLOAD.

My two cents: CR_UNLOAD is a very misleading name if I consider for what it's used.
When I close a library (whatever it means), the library itself is necessarily unloaded. In this case instead, closing and unloading are very different operations and the latter takes part to the game only during a reload.
This isn't clear from the documentation and the intuition I've from the computer science tells me quite the opposite.

If I got it, with cr I can have only this chains:

• load -> close
• load -> (unload -> load)* -> close

Where (unload -> load) means that the plugin has been reloaded. Right?
I think it's pretty clear if put in this terms.

[fungos]

Yes, that is also correct!
I can see the confusion, thanks a lot for bringing it up and really sorry about this oversight causing you any trouble.

I think my initial rationale was that in the context of hot-reloading, loading/unloading were specific to the hot-reloading part as these are specific steps required for data lifetime cross-instance. The open and closing of plugin does not incur in load and unload, but unfortunately I did not name the function cr_plugin_open (the one cr_plugin_load).

Also in the case of a cr_plugin_open, a new cr_op::CR_OPEN would be required for consistency (avoiding your first issue).

I think it is a bit late to rename these (if we can even find a better naming), but I'll also put that into the documentation (once I get back from vacancies next year).

I may consider further on renaming and deprecation steps.

[fungos]

Note that you can use CR_CLOSE as CR_UNLOAD in your situation, in the docs (at least?) we have this that hint at the difference:

• CR_CLOSE Like CR_UNLOAD but no CR_LOAD should be expected;

[fungos]

Hello, in the commit above I did some rewording in the doc and added a cr_plugin_open function to replace the old load. Take a look there please.If you have any suggestion to still improve the wording, I can change it more before merging.

[skypjack]

Oh, wow, thank you for taking in consideration my comments.
It looks good, open sounds definitely the right term and the doc now is clear about load/reload.

Just one question for curiosity: why did cr_set_temporary_path become a non-static function?

[fungos]

Yeah, that one is a bit out of place right now. It was triggering a warning as it is unused and is exposed in the doc as a public API. On the other hand, it is not extern neither a valid C function. So I just removed the static to silence the compiler, no other good reason. Not sure what to do with that one yet :)

[skypjack]

I see, yeah, I've those warning as well actually. Another one comes from a shadowing variable and I thought more than once to open a PR to suppress it too. 😄

[fungos]

Which one? :)
Feel free to open PR for anything that helps improve the quality, even typos in doc are worth it.

[skypjack]

Sure, thanks for inviting. It's a pleasure to work on your code actually, it's well written. 👍
The warning comes from this line:

auto ehdr = (Elf64_Ehdr *)p; // shadow

I've never opened an issue because I thought it was on purpose since you put there that comment.
The fact is that this triggers a warning when you instruct the compiler to detect shadow variables.