Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double call 'dofile' for nut with 'newslot' #242

Open
AlexandrG5 opened this issue Aug 18, 2021 · 11 comments
Open

Double call 'dofile' for nut with 'newslot' #242

AlexandrG5 opened this issue Aug 18, 2021 · 11 comments

Comments

@AlexandrG5
Copy link

AlexandrG5 commented Aug 18, 2021

Hi everyone!
If I call 'dofile' for the same 'nut' twice 'newslot' isn't called for the second time and all the exported methods (from the first call of 'newslot') are lost.
Is it OK?
I guess the problem is in this code.
bool SQVM::NewSlot(const SQObjectPtr &self,const SQObjectPtr &key,const SQObjectPtr &val,bool bstatic)
{
if(type(key) == OT_NULL) { Raise_Error(_SC("null cannot be used as index")); return false; }
switch(type(self)) {
case OT_TABLE: {
bool rawcall = true;
if(_table(self)->_delegate) {
SQObjectPtr res;
if(!_table(self)->Get(key,res)) {
Push(self);Push(key);Push(val);
rawcall = !CallMetaMethod(_table(self),MT_NEWSLOT,3,res);
}
}
if(rawcall) _table(self)->NewSlot(key,val); //cannot fail

@zeromus
Copy link
Contributor

zeromus commented Aug 18, 2021

Uhmm sounds to me like this is what you would want to happen? new methods replace the old ones in already-existing slots... maybe you should make a small test case to post here...

@AlexandrG5
Copy link
Author

'!_table(self)->Get(key,res)' return 'false' and 'CallMetaMethod' does't set the new methods so 'if(rawcall) _table(self)->NewSlot(key,val); ' set the empty slot instead old. As result I lose all the existing methods for this slot.

@rversteegen
Copy link

Calling dofile twice on a file works fine for me, in general. We can't understand what you are complaining about without seeing a testcase. What are you using a _newslot metamethod for? _newslot is only called if the slot doesn't already exist, it isn't called on every use of the <- operator.

@AlexandrG5
Copy link
Author

AlexandrG5 commented Aug 23, 2021

'What are you using a _newslot metamethod for?' - in _newslot I call
sq_pushobject(CScriptVM::GetHandle(), _Table/*HSQOBJECT*/);
_Key.Visit(details::SqPushVisitor());/*exported method's name*/
_Value.Visit(details::SqPushVisitor());/*method*/
bool Success = SQ_SUCCEEDED(sq_newslot(CScriptVM::GetHandle(), -3, _IsStatic ? SQTrue : SQFalse));
kdAssert(Success);
sq_pop(CScriptVM::GetHandle(), 1);

  • I push a new method to HSQOBJECT, but if I call dofile second time squirrel rewrite HSQOBJECT without my method because _newslot already called.

@zeromus
Copy link
Contributor

zeromus commented Aug 23, 2021

Now it sounds like you're saying that you want newslot to run when the slot isn't new. Try using _set and _get instead?

@AlexandrG5
Copy link
Author

My task was initially to solve an issue with exported methods deleting after the second call of 'dofile'. I just wonder whether this action is a bug. I found the place in the code of the virtual machine squirrel (see my first message) where I see the logic of a single call of 'newslot' for every 'nut' file and the deletion of already exported methods. This issue is possible to solve by changing the code above to this one:

bool SQVM::NewSlot(const SQObjectPtr &self,const SQObjectPtr &key,const SQObjectPtr &val,bool bstatic)
{
  if(type(key) == OT_NULL) { Raise_Error(_SC("null cannot be used as index")); return false; }
  switch(type(self)) {
  case OT_TABLE: {
    if(_table(self)->_delegate) {
      SQObjectPtr res;
      if(!_table(self)->Get(key,res)) {
        Push(self);Push(key);Push(val);
        if (!CallMetaMethod(_table(self), MT_NEWSLOT, 3, res)) _table(self)->NewSlot(key, val); //cannot fail
      }
    }
    else {
      _table(self)->NewSlot(key, val); //cannot fail
    }

Are these fix valid? Can you advise how to test it? (in case this was really a bug)

@zeromus
Copy link
Contributor

zeromus commented Aug 24, 2021

I still doubt anyone can understand what you're going on about without an example, which you haven't given. The most general way of using squirrel would be to have dofile define functions and if you run it again, the functions will be defined again. Sounds like that's what's happening for you. If you don't want that to happen, you have to take precautions to avoid it. It's unbelievable to me that you would try to modify such a key function in squirrel instead of understanding the way squirrel is meant to work.

@AlexandrG5
Copy link
Author

AlexandrG5 commented Aug 25, 2021

Example:

//SomeClass.nut
class SomeClass
{
  function SomeFunc1()
  {
    SomeFunc3();
  }
  function SomeFunc2()
  {
    SomeFunc4();
  }
}

//SomeClass.cpp
class SomeClass:
{
public:
  void SomeFunc3(){}
  void SomeFunc4(){}
};

When I call dofile first time I export in newslot to the script SomeFunc3 and SomeFunc4 but after the second time call dofile I lost SomeFunc3 and SomeFunc4 because newslot already called.

@zeromus
Copy link
Contributor

zeromus commented Aug 25, 2021

this example is not useful. Only you know how you're binding SomeFunc3 and SomeFunc4. Can you make an example that doesn't involve c++ binding?

@AlexandrG5
Copy link
Author

"Can you make an example that doesn't involve c++ binding?" - the problem reproduce with c++ binding only.
Example: how I add SomeFunc3 to the script from the c++ class in newslot.

sq_pushobject(CScriptVM::GetHandle(), _Table/*HSQOBJECT - SomeClass.nut class*/);
_Key.Visit(details::SqPushVisitor());/*exported method's name*/
_Value.Visit(details::SqPushVisitor());/*method SomeFunc3*/
bool Success = SQ_SUCCEEDED(sq_newslot(CScriptVM::GetHandle(), -3, _IsStatic ? SQTrue : SQFalse));
kdAssert(Success);
sq_pop(CScriptVM::GetHandle(), 1);

@zeromus
Copy link
Contributor

zeromus commented Aug 25, 2021

So you're saying when you attempt to bind SomeFunc3 a second time using this code, it doesn't work? What values end up being bound? The intended method for the 2nd time, the intended method for the 1st time, or nothing at all or something like null?

you keep vaguely sayingnewslot. Don't be lazy. Do you mean MT_NEWSLOT or sq_newslot or SQVM::NewSlot?

Honestly I don't think anyone will ever help you unless you make a complete demo with a standalone .c file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants