[ags4] add string Split and Join

[ericoporto]

adds String Split - it's working afaict. I am leaving as a draft because I would like to add String Join but I don't know yet how to read a String array.

[ivan-mogilko]

but I don't know yet how to read a String array.

String dynamic array is an array of managed handles; these are 32-bit values, which may be resolved to a String object using ccGetObjectAddressFromHandle.

[ericoporto]

I got String.Join to work!!!

String dynamic array is an array of managed handles; these are 32-bit values, which may be resolved to a String object using ccGetObjectAddressFromHandle.

Thanks for this because without it, I wouldn't get anywhere! I will leave as a draft still because I want to see if I can improve it's code still.

Edit: ah, wouldn't obsess too much, but if we had a global string.Empty object we could try to output a reference to it when possible instead of creating a new empty string.

[ericoporto]

I got code that works and I think the code in general is looking ok now - but any possible changes please tell me!!

Other things to review!

• Do we want this? I feel these are useful extensions, but it's good to check how everyone feels.
• is the API ok? Please check the names of things in the API, parameter order, ...
• @AlanDrake suggested adding a flag for Trim in String.Split too, but maybe this would complicate the code significantly - it would need to be trimmed before allocating the buffer but using a specific function that did not copy the buffer but instead selected things... I liked the idea at first but now I would prefer this to be done in AGS Script as a String extension
  • I feel some additional string stuff could be in a script library of sorts...
• Tests: my plan is to put the tests in ags script itself later, do I need to test something here in the engine tests?

[ivan-mogilko]

@AlanDrake suggested adding a flag for Trim in String.Split too, but maybe this would complicate the code significantly - it would need to be trimmed before allocating the buffer but using a specific function that did not copy the buffer but instead selected things...

I don't think it would complicate much. You are already using 2 pointers: for beginning of a string and end of string. Trimming the source would mean moving these pointers right and left correspondingly. I'm speaking about implementation, but don't have an actual opinion from API's perspective. Any additional arguments could be added later as well, with default values for backwards compatibility.

[ivan-mogilko]

There's a curious question about null arguments in Join. I noticed that passing null array would result in returning null pointer, but having null Strings in that array have same effect as empty strings. Which means that if I pass an array of only nulls, that would return an actual string, which may be empty or containing a sequence of separators.

Just a observation... maybe this is what's intended.

[ivan-mogilko]

Well, I find mostly minor things that may be optimized. I will be testing this properly tomorrow, but I guess it should be working.

[ivan-mogilko]

I need to honestly say that I will want to pick out an algorithm of resolving handles to object pointers as a separate function, that may be used in any API that has a dynamic array argument. I believe that's better to have this algorithm standalone rather than to mix it with data processing. Similar to how there's a helper function for converting a list of objects into dynamic array of handles.

[ericoporto]

If you write this function and put it somewhere in ags4 I can rebase and adjust the code to use it.

[ivan-mogilko]

If you write this function and put it somewhere in ags4 I can rebase and adjust the code to use it.

Okay, I wrote couple of variants, for cases when you do and do not need to know actual "managers": f5f5265a9edbc343495c99e5ac4a23a90ea684e7

---

EDIT: in regards to this PR, I tested that String.Join correctly recreates a string after String.Split, and it works as expected.

[ericoporto]

@ivan-mogilko I tried to adjust in the commit https://github.com/adventuregamestudio/ags/pull/2401/commits/99d408038b6912c2219e11f17c5a86e6ff988834

EDIT: in regards to this PR, I tested that String.Join correctly recreates a string after String.Split, and it works as expected.

This exact case is failing for me in my commit, the assert(header.IsPointerArray()); is failing, so I think I did something wrong building the string. :/

[ivan-mogilko]

I just tried your latest commit, and it still works for me.

Can you post the script you're using for a test?

EDIT: this is mine:

function game_start()
{   
    String full = "Split me by the space!";
    String split[] = full.Split(" ");

    for (int i = 0; i < split.Length; i++)
    {
        Display("%d: %s", i, split[i]);
    }

    String joint = String.Join(split, " ");
    Display("joint: %s", joint);
}

EDIT2: Ah, I noticed that you swapped the order of Join args in the latest version. Could it be the reason, in case you did not recompile the game but only the engine?

[ericoporto]

ah, crap, that is exactly it, I recompiled the game but did not place in the right folder...

Edit: added one more create array helper... https://github.com/adventuregamestudio/ags/pull/2401/commits/0bcad1111e3361f3c78e7b52508e05042fd62d10

I will stop creating these ones, but the idea here was that I was create an array using const char* with strings that could disappear because I wasn't adding reference to it.

I will reorganize the commits so that I add the helpers in their own commits, individually, and later use them.

[ivan-mogilko]

Please note, that you may also use CCDynamicArray::CreateOld if you don't want to bother with type names. For array of pointers you simply can pass "is_managed" as true.

[ivan-mogilko]

Could you elaborate the meaning of "fix scope crashes" commit?

Comment sais:

fix crash if the string goes out of scope but not the array

Do you refer to C++ code, or AGS script?

TBH I don't understand what the problem was, as CreateStringArray creates duplicates of strings...

[ericoporto]

Sorry, I am readjusting things, but the problem is if you write in AGS Script something like this:

  String splitted[];
  {
    String str = "text,to,split";
    splitted = str.Split(",");
  }
  for(int i=0; i<splitted.Length; i++)
  {
    System.Log(eLogInfo, "'%s'",  splitted[i]);
  }

TBH I don't understand what the problem was, as CreateStringArray creates duplicates of strings...

uhmm... maybe I rushed too much and didn't realize this and had a crash in other thing...

I will do a final push force to adjust things in their own commits and then let this sit for a while.

[ivan-mogilko]

The PR looks fine overall now. One thing I may mention: since you added CreateArray helper, you could just use that instead of CreateStringArrayFromBuffers, if you'd create ScriptStrings from these buffers right away.

Anyway, there's a room of improvement in the existing helper functions, and string factory methods. For example, ScriptString::Create could take an optional copy_len argument, limiting the portion of the copied string, which would let initialize it with a substring too.

EDIT: I don't have time to test this thoroughly with script right now, but I'll try few things a bit later today.

[ericoporto]

There's no rush to this and it doesn't block anything, so it's ok to take time. There was also no previous issue discussing it.

[ivan-mogilko]

Well, I tried few script tests, and things appear working, including returning arrays from functions, splitting by multiple char sequences, etc. I may merge this now, and leave any possible code simplification for the later.