Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behavior of foreach with empty elements in list #519

Closed
winterfrost1 opened this issue Nov 8, 2013 · 9 comments
Closed

Behavior of foreach with empty elements in list #519

winterfrost1 opened this issue Nov 8, 2013 · 9 comments
Labels
Question User has a question, not an issue with Denizen.

Comments

@winterfrost1
Copy link

I don't know if this is the desired behavior for foreach and empty elements, but it struck me as odd.

Example:
I create a sign at l@signlocation that says:
Test
[BLANK]
[BLANK]
[BLANK]

And I create a script (excerpt):

- define signlocation l@%locx%,%locy%,%locz%,<npc.location.world.name>
- define signcontents %signlocation%.sign_contents
- narrate "Size of sign list <%signcontents%.size>"
- narrate "Sign line 1 <%signcontents%.get[1]>"
- narrate "Sign line 2 <%signcontents%.get[2]>"
- narrate "Sign line 3 <%signcontents%.get[3]>"
- narrate "Sign line 4 <%signcontents%.get[4]>"
- narrate "Now to loop..."
- foreach <%signcontents%> {
  - narrate "This line says %value%"
  }

And I get:

Size of sign list 4
Sign list 1 Test
Sign list 2
Sign list 3
Sign list 4
Now to loop...
This line says Test

Shouldn't foreach loop through all elements in the list, even if they're empty/blank?

Thanks!

@mcmonkey4eva
Copy link
Member

This is the doing of some internal dList code, not foreach specifically.
This is probably the desired behavior... though I don't know why or where exactly it's caused at.
@aufdemrand - ^ ? Know anything specific on that?

@aufdemrand
Copy link
Contributor

So are you saying that you think a sign should always return a 4-item list? That probably makes sense, since you could use line 1 and line 3, and denizen might not maintain that the second item in the list was on line 3. Maybe empty lines could return 'null'?

@mcmonkey4eva
Copy link
Member

@aufdemrand - but then how would you know if the player just literally typed 'null' onto the sign?
I think either somehow making empty elements or just letting the list shrink to only the existing lines is the way to go.
It can handle blank elements if they're in the middle (EG li@one||three), why not at the end?

++ get[4] if line 4 doesn't exist already returns null...

@aufdemrand
Copy link
Contributor

Well first if all, || will be interpreted as an 'alternative'. Null is
already a phrase to avoid within Denizen when using for anything but the
literal use of something being 'null'. Thoughts?
On Nov 12, 2013 9:52 PM, "mcmonkey" notifications@github.com wrote:

@aufdemrand https://github.com/aufdemrand - but then how would you know
if the player just literally typed 'null' onto the sign?
I think either somehow making empty elements or just letting the list
shrink to only the existing lines is the way to go.
It can handle blank elements if they're in the middle (EG li@one||three),
why not at the end?


Reply to this email directly or view it on GitHubhttps://github.com/aufdemrand/Denizen/issues/519#issuecomment-28358584
.

@mcmonkey4eva
Copy link
Member

@aufdemrand - I wish we could use a stronger typing system, and have null not be the same as string "null"... but we don't and kinda can't so...
I guess the currently functionality is least error prone and thus the best we can use until/unless other parts of dScript's functionality change.

It's not too hard to use... you can set .get[4] and see that it's null, if you're worried about players literally typing 'null', you can just check the .size.

-- actually, || at the end isn't too much of a risk, even with alternatives. It causes no problems when it's in the middle (and we can't really delete empty elements from the middle - that would alter the list itself)

@winterfrost1
Copy link
Author

Hi guys, I think there's too much focus on this being a sign. I unfortunately can't test this right now, but from what I saw with the sign list I made the assumption that null values in an dList are simply being ignored/not looped through by the foreach command. For a sign, you're right, it's certainly no big deal to check each index and get the four values individually--that's how I'm working around this now--but I logged the call because i thought it might have more far reaching effects. In the case where you have an unknown number of elements in your dList, it seems illogical to me to "skip" some. What if you want to have an if statement in your loop to conditionally handle nulls? If the foreach statement doesn't even loop through the entries because they are null and appear at the end of the list, you can't do this. As far as checking the size of the list, that would work if you could do a while loop or a for... until and increment a counter to get each element, but I don't think Denizen can do either of those currently, can it? I haven't done professional development in many years (and even then I would never claim to have been particularly good at it! :D), but there's something logically wrong with a foreach loop that functions more like "forsome depending on the values" :)

If I'm completely wrong and this is specific only to a sign-generated 4-element dList, then please ignore me!

@mcmonkey4eva
Copy link
Member

@winterfrost1 it's 'for each element in the list'. All of them. The issue here is empty elements are removed from the end (only the end) of the lists themselves.

Also:

# Assume 'LSIZE' is 4 - the total size of the list
# Assume 'LIST' is li@one|two|| - a list that should have four items but the last two are empty and it's trimmed to just li@one|two
# Anything not in capitals is meant literally
- repeat <LSIZE> {
  - narrate "List[%value%] = <LIST.get[%value%]>"
  }

Outputs

List[1] = one
List[2] = two
List[3] = null
List[4] = null

foreach is pretty much just a shorthand for:

- repeat <LIST.size> {
  - define value <LIST.get[%value%]>
  # Code here, EG - narrate "%value%"
  }

So basically... if you want to handle empty elements are the end of a list, all you currently need to do is maintain a separate variable containing the total theoretical size of the list, and use that rather than foreach or <LIST.size>

... but I still support lists being allowed to retain empty elements at the end.

-- ALSO, an empty element is not the same thing as a null element. Empty = li@one|two||, Null = li@one|two|null|null

... actually, I'm not entirely sure when/where it's getting removed. It might be from the argument parser... which means it's only being dropped when you directly input it into foreach and <LIST.size> is still accurate.
If it is the argument parser, I'll see if I can workaround that for foreach...

@mcmonkey4eva
Copy link
Member

... and with that pull request I made, if it's accepted, there is 0% risk of || within a list being misread as OR, which just leaves tag-alternatives... which isn't too much of a problem, it only happens if you double tag.
(EG, if you do <<global.flag[potato]>> where potato = a list like li@3||x which is a pretty bad idea anyway)
... that can be fixed too, though...
EDIT: Fixed that too. Very minimal risk of issues with alternatives (Unless you type in <li@3||x> which is wrong anyway so you deserve what you get.)

@mcmonkey4eva
Copy link
Member

This was never really an issue when it was made, and all possible side-issues have been fixed...
Also it's gone stale. Closing for now.
If you have more questions / suggestions related to this thread, please reply and we'll re-open it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question User has a question, not an issue with Denizen.
Projects
None yet
Development

No branches or pull requests

3 participants