Object components in script

Not logged inOpenClonk Forum

Forum Home Help Search Watchlist Register Login

Topic Development / Developer's Corner / Object components in script

Post

By Maikel Date 2016-04-30 22:22

I moved object's Components from the engine to script in this series of commits. This allows overloading most of all and kills some of the useless functionality that was in the engine.

I'll merge into master tomorrow if there are no complaints. Yes, I know it is not backwards compatible.

By Zapper

Date 2016-04-30 22:45

The only thing that is not backwards compatible is that the material definition went from DefCore to script, right? The interface (GetComponents/SetComponent e.g.) is still the same? (Appears like that from a glimpse at your commits)

By Sven2

Date 2016-05-01 02:10

Does this still integrate with Con? I.e. do small trees have less wood?

Also, it would be cool if components could just be a proplist (aka Dictionary). I.e. this.Components[Wood] += 1 instead of looking through an array.

By Maikel Date 2016-05-01 08:56 Edited 2016-05-01 12:34

>Does this still integrate with Con? I.e. do small trees have less wood?

I have to reimplement that. Edit: done. Do objects which oversize even get more components?

>Also, it would be cool if components could just be a proplist (aka Dictionary). I.e. this.Components[Wood] += 1 instead of looking through an array.

Exactly this notation? Or Components.Wood? Or Components["Wood"] (is the same)? I think I prefer this.Components.Wood then, but what is the advantage of such a system apart from being cleaner?

Edit: I have an implementation which works with: local Components = {Wood = 1, Metal = 4, Rock =1}; now. It that good?

By Sven2

Date 2016-05-01 14:11

Well, Components["Wood"] sucks because you cannot do e.g. foo.Components(bar->GetID()). Converting an ID to a string is always a bit awkward (even though in the engine, I think it already knows its C4String *).

I wonder if we could change proplist access to work with ids and auto-convert them to strings?

By Maikel Date 2016-05-01 14:23

You can do foo.Components.Wood = 1; or foo.Components[Format("%i", bar->GetID())]; but there is the SetComponent wrapper for that. I am going to stick with Components = {Wood = 1} for now.

By Zapper

Date 2016-05-01 15:17

What's the advantage? You can't do the obvious things where that would actually be useful (e.g. this.Components[o->GetID()] -= 1). You can use (hardcoded) this.Components.Wood, yeah. But I believe there is not one place in the repository where that would be used right now..? I don't really count that as an advantage.

But the big disadvantage that makes me oppose this is another one.
Imagine you had Components = [[Nugget, 1]] and someone renamed the "Nugget" to "Gold". You'd then get an error along the lines of "undefined identifier 'Nugget'".
Okay, now imagine you had Components = {Nugget = 1} and someone renamed it. You'd get..., yeah, nothing at all. Because the thing in the proplist is not an ID but a string. So it would silently fail and noone would notice.

And what would GetComponent return? Are you casting the string to an ID every time that function is called?

By Maikel Date 2016-05-01 15:32

I find that a very small disadvantage, at some point you notice the missing component and fix it, the game actually remains playable.

GetComponent returns nil, and id or a number.

By Zapper

Date 2016-05-01 15:49

>I find that a very small disadvantage, at some point you notice the missing component and fix it, the game actually remains playable.

Yeah, probably after the release when some round doesn't work and some users are again frustrated and decide to leave the game alone because something doesn't work. At least that's not untypical when watching our Let's Plays.

What's the advantage? You save a bracket?

By Maikel Date 2016-05-01 15:55

It's potentially faster. And for the main game content it does not make a difference. If you replace an ID by a new one and do not search for all its occurrences, well, then one is an *****.

By Zapper

Date 2016-05-01 15:58

>It's potentially faster.

Is it?
So you are not casting the string to an ID every time someone calls GetComponents? And you are not first getting the dictionary keys as an array of strings when iterating over the indices? (And hopefully that not for every call of GetComponents, too).

>If you replace an ID by a new one and do not search for all its occurrences, well, then one is an *****.

Well, it's not an ID in your case. It's a string.
Also, I have quite a few custom things lying around. And if something breaks due to changes in the original pack I would rather know that when starting a round and not "at some point".

By Maikel Date 2016-05-01 16:03

GetComponent may be slower and SetComponent may be faster.

The custom packs do throw defcore errors :)

By Zapper

Date 2016-05-01 16:26

>The custom packs do throw defcore errors :)

Yes, now. But I mean in the future when something changes. Not from the old system to the new one (which happens exactly once and is OK) but possibly in every future version.

>GetComponent may be slower and SetComponent may be faster.

That is completely true, but

David@david-desktop /c/OpenClonk/planet/Objects.ocd
$ grep GetComponent -r ./* | wc -l
27
David@david-desktop /c/OpenClonk/planet/Objects.ocd
$ grep SetComponent -r ./* | wc -l
0

So I'd say stating "It's potentially faster" is kind of misleading :)

By Marky

Date 2016-05-01 07:39

Yes, I would like to merge the liquid container branch before you do this. There are a lot of unit tests for it, but so far nobody has actually tested it (that is, unless you used liquids in Classics.ocf, because that uses the liquid container branch).
This should introduce a few new objects and changes the components of some objects so that their components are liquids. I think it will be easier for you to merge the changes than the other way round.

By Maikel Date 2016-05-01 08:58

Okay, then I do it after my holidays in a few weeks.

By Günther [de]

Date 2016-05-01 18:03

Have you updated tools/convert_cr_to_oc.py for this? There's no need to make everyone manually update. Also the engine could do this - C4DefScriptHost::Parse() already sets the Plane, it could set the new property, too. Then we just need to add deprecation warnings to the defcore parser, and can remove the old thing in a release or two without breaking everything.

By Maikel Date 2016-05-01 19:54

I'll have a look into this.

By Maikel Date 2016-05-02 09:34

The convert tool just "destroyed" my repos by adding empty script files... so I am not going to invest time into that.

By Günther [de]

Date 2016-05-03 01:15

Preventing empty script files was be a trivial change, not that they would have caused any trouble.

By Maikel Date 2016-05-01 19:53

Open question: should trees with SetCon(150) give more wood than trees with SetCon(100)? I.e. should components scale above the nominal values for objects which have OverSize=1?