Accessing AGS objects by array index / Hardcoding editor values in code

Started by Billbis, Wed 26/06/2013 09:57:09

Previous topic - Next topic

Billbis

I have no idea of what will be the best solution, but I just wanted to enphasize that we should keep a way to quickely iterate things across all (existing) regions, like what we can do now :
Code: AGS
int i =1;
while (i < 50) { //current hostpot limit
    //do stuff on hotspot[i]
    i++;
}

But maybe there is no need to remind this to you, gods of code. ;)

(Split from here. - moderator)
(sorry about derailing the topic on the first reply.  :( - Billbis)

Crimson Wizard

Quote from: Billbis on Wed 26/06/2013 09:57:09
I have no idea of what will be the best solution, but I just wanted to enphasize that we should keep a way to quickely iterate things across all (existing) regions, like what we can do now
There's no reason why this may become unusable...
What changes is actual number of hotspots in room, i.e. you will need to write:
Code: ags

while (i < Room.HotspotCount) 

Billbis

Quote from: Crimson WizardThere's no reason why this may become unusable...
What changes is actual number of hotspots in room, i.e. you will need to write:
Yes, but that will somehow require to rearrange hostopt.ID when user is deleting one hotspot, wich can be ... disturbing.
But anyway, as long as I can iterate through all hostpots, I will be happy.  ;-D

Crimson Wizard

#3
Quote from: Billbis on Wed 26/06/2013 10:13:36
Yes, but that will somehow require to rearrange hostopt.ID when user is deleting one hotspot, wich can be ... disturbing.
As well as deleting a Character, or a GUI. That's being a problem for a long time.
But using explicit ID may be replaced with script name, which is far more safe.
Range operations and calculating relative id will still work so far as you can address array of hotspots from script.

Ryan Timothy B

Quote from: Crimson Wizard on Wed 26/06/2013 10:31:56
But using explicit ID may be replaced with script name, which is far more safe.
I'm really not sure why being able to access the "Object" ID was ever implemented in AGS. Likely due to the limitations of AGS script and then trying to stick with the whole backwards compatibility - which in my opinion is only hindering all radical improvements of the editor.

MiteWiseacreLives!

Quote from: Ryan Timothy on Wed 26/06/2013 14:50:22
I'm really not sure why being able to access the "Object" ID was ever implemented in AGS. Likely due to the limitations of AGS script and then trying to stick with the whole backwards compatibility - which in my opinion is only hindering all radical improvements of the editor.
I've used the object[ID] to apply settings to a large number of objects etc during runtime, kind of like Billbis is describing. Not so handy with script names, unless I am overlooking a method.

Crimson Wizard

#6
Quote from: MiteWiseacreLives! on Wed 26/06/2013 15:29:05
I've used the object[ID] to apply settings to a large number of objects etc during runtime, kind of like Billbis is describing. Not so handy with script names, unless I am overlooking a method.
Indeed, working with ranges of elements is important possibility, and its not going anywhere.
Although I am not sure if Ryan meant accessing elements of array; I thought he means accessing Object.ID property.

Anyway, I started this topic for completely different reasons, and I think it is better to continue the discussion on IDs in separate thread.

I really need to figure out how to update user interface. This is pretty irritating, 'cause the code for unlimited regions is practically ready.
So far I can only imagine adding +/- buttons on the topmost panel of Room Editor, as a temporary solution for Alpha version.

Ryan Timothy B

#7
Quote from: MiteWiseacreLives! on Wed 26/06/2013 15:29:05
I've used the object[ID] to apply settings to a large number of objects etc during runtime, kind of like Billbis is describing. Not so handy with script names, unless I am overlooking a method.
Quote from: Crimson Wizard on Wed 26/06/2013 15:45:12
Indeed, working with ranges of elements is important possibility, and its not going anywhere.
Although I am not sure if Ryan meant accessing elements of array; I thought he means accessing Object.ID property.
Nope. I meant exactly what I said. Accessing "Objects" by a strict ID is a terribly, terribly poor coding practice ("Objects" being GUIs, Characters, Objects, Hotspots, Regions, WalkableAreas, etc). They should be accessible via an ArrayList (which of course is the same thing, but you shouldn't have Character.ID to find what index it is in the ArrayList).

Code: ags

  // Character variables created in the accessible "Game" script
  Character cHenry = new Character("Henry", 100, 150, 5);  // Name: Henry, X: 100, Y: 150, Room: 5
  Character cSally = new Character("Sally", 110, 160, 6);  // Name: Sally, X: 110, Y: 160, Room: 6
  Character cFrank = new Character("Frank", 120, 170, 7);  // Name: Henry, X: 120, Y: 170, Room: 7
  
  // Characters being added to the AGS character ArrayList
  Game.Characters.Add(cHenry);
  Game.Characters.Add(cSally);
  Game.Characters.Add(cFrank);

  // Getting the index of cSally - if you ever needed it, which you shouldn't in most cases
  int sallyIndex = Game.Characters.IndexOf(cSally);

  // iterating through the Characters ArrayList
  foreach (Character c in Game.Characters) {
    c.Visible = false;
  }

  // iterating through the Character ArrayList as it is now
  int i;
  while (i < Game.Characters.Length) {
    Game.Characters[i].Visible = false;
    i++;
  }


Etc etc. You understand how ArrayLists work. This is how AGS should've been designed, without the strict IDs. And yes, I think it's very relevant to the discussion of adding unlimited regions, as even regions should be Dynamic. If you're going to do it, may as well do it right.

Crimson Wizard

Quote from: Ryan Timothy on Wed 26/06/2013 22:56:56And yes, I think it's very relevant to the discussion of adding unlimited regions, as even regions should be Dynamic. If you're going to do it, may as well do it right.
Dynamically created entities were in plans, but I wasn't intending to do this right away. I can't tell for sure how much more time this will take, and I have no idea if I will find time (and enough devotion) to work on this.
Meanwhile few people already asked me personally for a version with increased limits of objects. So here it goes.
And while what you say may be related to unlimited regions, this topic is not about unlimited regions. It is about Editor UI. (Not any more, it isn't. - moderator)


EDIT: removed overly emotional line.

Calin Leafshade

#9
Quote from: Ryan Timothy on Wed 26/06/2013 22:56:56
Nope. I meant exactly what I said. Accessing "Objects" by a strict ID is a terribly, terribly poor coding practice ("Objects" being GUIs, Characters, Objects, Hotspots, Regions, WalkableAreas, etc). They should be accessible via an ArrayList (which of course is the same thing, but you shouldn't have Character.ID to find what index it is in the ArrayList).

Says who?

There's nothing intrinsically wrong with using integer IDs as part of an array. Providing the characters are also available as global, named symbols it doesn't matter.

Also, beware foreach loops. They have sneaky caveats, especially in game programming.


Ryan Timothy B

Says me obviously.  :P  If you ever introduced a delete method to delete an "Object" it would reek havoc on any code you have. Whereas the currently deleted character index would be swapped with the last index (or having the whole array sort itself to remove the deleted index).

The only reason I believe it's even useful to have "Object".ID at this very moment is because we don't have support to extend to the current AGS structs yet.

Crimson Wizard

#11
I had an argue about IDs with Iceboty 7000 about a year ago, and funny thing is, back then I was on Ryan's place.
(I may try to find the thread later)

However I quickly learnt there are cases when you must have a way to iterate through part of array, or take out an item by its relative position.
Of course there are ways to do this without numeric ids, but they may be too difficult for usual user of AGS. Any way, this is absolutely not a simple matter.


E: Ah, here you go:
http://www.adventuregamestudio.co.uk/forums/index.php?topic=36835.msg627199#msg627199

tzachs

Quote from: Crimson Wizard on Wed 26/06/2013 22:32:12
tzachs, if I understand you right, what you say sounds like a simplified version of my old suggestion
Yes, I guess you can say that..

Quote from: Calin Leafshade on Wed 26/06/2013 23:21:03
Also, beware foreach loops. They have sneaky caveats, especially in game programming.
What? Care to explain that?
The opposite is true, as far as I know, at least in c#. By using foreach you allow the compiler to make optimizations so it'll be more efficient.
The only scenarios which you'd prefer to use 'for' is if:
1. You have an interest in the index as well as the object you're iterating on.
2. You want to remove items from the collection while iterating on it, then you'll do an opposite for (from end to start).


Calin Leafshade

Quote from: tzachs on Thu 27/06/2013 08:50:56
What? Care to explain that?

foreach loops create extra work for the garbage collector because they create extra references. This can be especially problematic in game programming because it can potentially create several thousand new references every second (a particle system perhaps). Also the .NET GC is non-deterministic so you can never be sure exactly when the GC will do its work which can create random usage spikes which can severely fuck with your frame rate, especially in embedded systems like the Xbox360 or a phone.

Ryan Timothy B

Oh right. I remember reading about that on the LibGDX forum.

Snarky

I agree with Calin in that there's nothing inherently wrong with using an integer index to access data objects, and that it's important to have a way to loop through multiple objects. Using an iterator in a foreach loop may be a bit more elegant, but it doesn't really improve the program logic in any way, and as pointed out is has implementation drawbacks.

In any case, AGS script is pretty far from supporting something like that. We don't have ArrayLists, or any good way of implementing them. Hell, we don't even have for loops! Maybe deal with that first, before jumping ahead to foreach? In the mean time, the built-in arrays offer a simple and perfectly workable way to iterate through the different data objects. It's not "sticking to backwards compatibility" to preserve a useful feature when a possibly superior alternative remains out of reach for the time being.

Ryan Timothy B

I believe me adding the foreach in that code snippet has confused you folks (I'm not actually sure why I even included that into my example in the first place). I never meant this conversation to argue that accessing "Objects" by array was bad coding practice - who the hell would want to abolish an array via index? I was stating that getting the index the "Object" (also via "Object".ID) shouldn't be hardcoded AT ALL.

For instance: when you add a character in the tree, it shouldn't be
0: cGeorge
1: cSally
2: cFrank

Because when you are scripting, you can go crazy typing character[1] all over the place and know it'll always be cSally. But if AGS were to one day improve itself, and one were to ever delete cSally, or insert a new character before cSally, then goodbye to having character[1] point to cSally.

And I apologize for this to have derailed your thread, Crimson. I agree to the topic split.

Snarky

I don't really see your point, Ryan. As long as the array index is the best way to iterate through multiple AGS objects, (or to do various forms of "pointer arithmetic"), we need a way to tell which index corresponds to which object. If you write some code that affects some subset of characters, for example, you probably want to check that the range is the right one. If you don't have it in the editor and not as a field, how are we supposed to use it at all? Iterate through the array until we find a match?

Sure, you shouldn't go around referring to a known character by its index (since, as you say, the link between index and object is less immediate and less stable), but that's a matter for good coding practice, not an argument for removing the engine or editor feature.

Ryan Timothy B

#18
Quote from: Snarky on Fri 28/06/2013 12:12:58
I don't really see your point, Ryan.
I know you aren't. I don't know why you don't see the point, which is why I'll have to explain it in full depths. (laugh)

Let me walk you through this so you understand in clear logic. In AGS create 2 GUIs, 2 buttons, 2 dialogs, 2 characters, 2 objects, etc. Then access these elements via script referring to them by the very specific ID AGS allows you to believe they are. For example: your first character is Character1, second is Character2. The Character ID's are as follows:

Character1 = character[0]
Character2 = character[1]

Or as I said above:
Quote0: cGeorge
1: cSally
2: cFrank

You know that when you type the following it will always point to the Character1, because AGS tells you it will.
Code: ags
character[0].Visible = false


Now after you've added all that code pointing to character[0] or any of those multiple "Objects", delete it from the editor.

My argument is that it is using hardcoded numbers in the EDITOR, making you feel safe that that's the ID number to use, when in fact it ISN'T. Deleting that numbered "Object" only reorganizes all other objects, therefor making the constant ID mute and a TERRIBLE design for the editor. If I still don't have a bandwagon of followers, I'll have to post a double face palm picture or something of the sort.  (laugh)

Now in a future version of AGS where everything is Dynamic, the "Object".ID can still actually stay, just have the Get function access the ListArray like in the code example I posted above. Having it find the first and only matching object in the array, telling you the index. Or you can go the arduous route of an integer variable within every object, but that requires a change every time other objects get a new order - which is a much heavier issue to deal with and more prone to errors on the editor/engine scripting side of things.

Just so we're clear, I'll say it one last time to be sure. AGS shouldn't tell you what ID the "Object" is in the tree. If ANYTHING, it should be assigned during compile time. Thank-you for your time.

Also that's why I posted this in the Editor thread, but that's fine.. It can always get moved back. But now that I've reread my posts, I suppose my arguments have been half and half on both matters.

Edit: I'm just really terrible at clearly pointing out and describing what I thought was a very known caveat about the editor.

Snarky

Quote from: Ryan Timothy on Fri 28/06/2013 17:08:49
You know that when you type the following it will always point to the Character1, because AGS tells you it will.
Code: ags
character[0].Visible = false


Now after you've added all that code pointing to character[0] or any of those multiple "Objects", delete it from the editor.

My argument is that it is using hardcoded numbers in the EDITOR, making you feel safe that that's the ID number to use, when in fact it ISN'T. Deleting that numbered "Object" only reorganizes all other objects, therefor making the constant ID mute and a TERRIBLE design for the editor. If I still don't have a bandwagon of followers, I'll have to post a double face palm picture or something of the sort.  (laugh)

Well, I get what you're saying now, but you're making an IMO unreasonable leap from "AGS displays this value and I can use it in my script" to "this value will never change, no matter what changes I make to my project."

Of course, in your code example there's no reason not to refer to the character explicitly by scripting name, but imagine that you wanted a feature in your game to put hats on any character. One way you might implement this would be to make each hat a character and have them follow the "real" characters around. If we pretend FollowCharacter() doesn't exist, you might code it something like this (simplified example):

Code: AGS

#define HATTABLE_START 15
#define HATTABLE_COUNT 8
#define HAT_START      328

function repeatedly_execute()
{
   int i=0;
   while(i<HATTABLE_COUNT)
   {
      if(character[HAT_START + i].Room != character[HATTABLE_START + i).Room)
         character[HAT_START + i].ChangeRoom(character[HATTABLE_START + i).Room);
      character[HAT_START + i].x = character[HATTABLE_START + i).x;
      character[HAT_START + i].y = character[HATTABLE_START + i).y;
      i++;
   }

   // ...
}


Where the defined values are the character indexes used as slots in the editor. OK, if I delete a character I might have to change these values (one reason I "never" use magic numbers in the body of my code; I always try to define them with meaningful names at the top of the script), but hey, that's development for you!

If you took away this feature, how would you deal with situations like this?

SMF spam blocked by CleanTalk