CopyTable() improvement

Feature requests for future versions of Hollywood can be voiced here
Post Reply
User avatar
Allanon
Posts: 732
Joined: Sun Feb 14, 2010 7:53 pm
Location: Italy
Contact:

CopyTable() improvement

Post by Allanon »

Hi Andreas,
I think it could be useful an improvement of the CopyTable() function to allow us to choice to make a strict copy (like it's working right now) or to make a referenced copy of the subtables.

This come useful for example to handle structures that can trigger recursive copy and make Hollywood crash.

For example I've encountered this problem during HGui and ScuiLib development and now, that I'm writing a simple sprite engine, too because my sorted sprite list reference the sprite objects and the sprites reference the sprite sorted list (to allow multiple lists), this way the sprite cloning using CopyTable() just make Hollywood crash.
I'm handling this problem manually so there is no problem for me, but I think it could be nice to have, at least you could insert a maximum recursion level to let Hollywood does not crash when this problematics are raised.
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: CopyTable() improvement

Post by airsoftsoftwair »

Hmm, do you have a small code example that makes Hollywood crash on CopyTable()? That is of course a bug and must be fixed.

Concerning the referenced copy: Hollywood never really makes real copies of tables. Internally, it all works by reference so I'm not quite sure whether I understand your problem correctly...
User avatar
Allanon
Posts: 732
Joined: Sun Feb 14, 2010 7:53 pm
Location: Italy
Contact:

Re: CopyTable() improvement

Post by Allanon »

Hi Andreas, here it is a simple and pointless example that shows how Hollywood crahses with CopyTable() when it find recursive/self-referencing tables. I understand that it could be a bad practice but sometimes is usefull to have this kind of referencing, anyway here is the code:

Code: Select all

Global myEngine = { a = 10, b = 20, ObjList = {} }
Global myEngineObj = { d = 20, e = 30, EngineRef = {} }


Function MyEngine:Create()
   Return(CopyTable(myEngine))
EndFunction

Function MyEngineObj:Create(engine, d, e)
   ; Create and add an item to the engine
   Local NewObj = CopyTable(myEngineObj)
   NewObj.d = d
   NewObj.e = e
   NewObj.EngineRef = Engine

   InsertItem(Engine.ObjList, NewObj)

   Return(NewObj)
EndFunction

Function MyEngineObj:Clone()
   Return(CopyTable(self))
EndFunction

; NewEngine
Local NewEngine = myEngine:Create()

Local NewObj1 = MyEngineObj:Create(NewEngine, 0, 20)
Local NewObj2 = MyEngineObj:Create(NewEngine, 0, 20)

Local Clone1 = NewObj1:Clone()

WaitLeftMouse()
As you see when I try to copy the table in the Clone() method Hollywood crashes because it goes (I presume) into an endless loop between the Engine field of the object and the ObjList of the MyEngine class that contains objects with references to the Engine itself and so on...
I've tried it under Windows
User avatar
nicholas.jj.taylor
Posts: 9
Joined: Thu Oct 03, 2013 11:28 am
Location: Harworth and Bircotes, Doncaster, Nottinghamshire, UK

Re: CopyTable() improvement

Post by nicholas.jj.taylor »

What happened under Windows?
A1200 + Blizzard MC030@50Mhz 128mb + MC68882 FPU (SCSi kit and +128Mb to come!)
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: CopyTable() improvement

Post by airsoftsoftwair »

@nicolas: Thanks for bumping this thread, I completely forgot about it.

@Allanon:
Hollywood will now no longer crash on a stack overflow in CopyTable() but exit cleanly. I've also implemented a second optional argument which allows you to tell CopyTable() to make a 'shallow' copy of the table, i.e. all subtables will only be copied by reference then, CopyTable() won't make independent copies of them. This solves the problem with copies of self-referential tables.
User avatar
Allanon
Posts: 732
Joined: Sun Feb 14, 2010 7:53 pm
Location: Italy
Contact:

Re: CopyTable() improvement

Post by Allanon »

Great! Thank you! :)
Post Reply