JBenda / inkcpp

Inkle Ink C++ Runtime with JSON>Binary Compiler
MIT License
70 stars 13 forks source link

Access global Ink variables [v2] #14

Closed JBenda closed 3 years ago

JBenda commented 3 years ago

Enables reading and writing of INK global variables from C++ code.

Use template function in class global to access them.

class {
    // @brief access global variable from ink story
    // @param name name of variable (as defined in Ink Script)
    // @return nullptr if there is no such variable
    // @return pointer to variable
    template<typename T>
    T* get_global(const char* name);
}
JBenda commented 3 years ago

So here we are again ^^

There is still the thing with strings, a read access is no problem, therefor we can just stash them and handle the pointer, a write access is quite a tricky thing I would postpone to a later point.

I will think about it the next few days, but I am not optimistic. Maybe a String access class which only give read access, but you can ask to privilege it to write access (which then will make a static string to an allocated and then edit this, but what happens when you want to set it to a static string, than you are not allowed to free it later on, etc. [maybe a maximal string size? -> no reallocation, but memory cost])

brwarner commented 3 years ago

I'm fine with string writing being added later. There's a bunch of holes in the runtime right now concerning strings. Thinking about how to implement it though...

I think you're right about returning some kind of special "container" class. It would support read-access (maybe via overloading []/==/etc.). For writing, maybe we'd just overload the assignment operator. When you assign, it asks the globals object to create a new string using its string_table (not sure if you've looked into it yet, but it's how I deal with dynamically allocating strings during runtime when it's necessary to do so) and then throw that pointer into the variable.

You don't have to worry about GC with the string table. The runtime periodically runs through all variables and deletes any entries in the string table it can't find in any of them.

I think we just wouldn't allow write access that wasn't assignment. I can hardly see a use case where someone wants to read a variable and just muck with a few of its characters. If they really want to do that, they can copy the value into a std;:string or something and just assign it back to our special string container.

JBenda commented 3 years ago

That's sounds good, I will try it this week ^^

JBenda commented 3 years ago

It's an idea of how it can work, it's only a scratch now, but it seems to work

brwarner commented 3 years ago

Wasn't sure if this was ready for review yet but I went through it and only really found one issue.

JBenda commented 3 years ago

No problem, I would wrap it up this weekend. I always appreciate when someone else (especially when he knows the code base better) take a look at my code. But often I hesitate to ask, because it is a time investment. So I'm very grateful that you review so frequently, thank you.

brwarner commented 3 years ago

But often I hesitate to ask, because it is a time investment

Just leave a comment saying you'd like a review before proceeding and I'll do it when I can. Don't worry about sounding pestering. You're making this project move faster, not slower. I actually need to dedicate less time to it because of you :)

JBenda commented 3 years ago

This should be functional know :) Pls check if I don't have done something silly.

brwarner commented 3 years ago

Cool. I'll take a look tomorrow :)

brwarner commented 3 years ago

If you fix the merge conflict in value.cpp I'll merge the branch in.

JBenda commented 3 years ago

Thank you for the review. The FIXME slipped me ^^

The branch is now rebased to master.