albertodemichelis / squirrel

Official repository for the programming language Squirrel
http://www.squirrel-lang.org
MIT License
894 stars 148 forks source link

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

Open AlexandrG5 opened 2 years ago

AlexandrG5 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

'!_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 commented 2 years ago

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 commented 2 years ago

'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);

zeromus commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

"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 commented 2 years ago

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.