Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Modifying objecct properties inside of an array?

I'm working with Twine 1.4x and Sugarcube 2. I have a menu where the player can install parts to a machine; all parts are objects containing the boolean properties 'have' (whether or not the player has it in their inventory), 'req' (whether or not the player has the requisite skill to install the part), and 'done' (whether or not the part has been installed). At the start of the install passage, checks are run for each part to see if the requirements have been made, and if so, to change the 'req' property to true. It then pushes every part to array '$genParts'. The install menu consists of a list of parts in the player's inventory as links; on click, if 'req' is false, the player gets an alert dialog saying they don't have the skill needed to install the part. If 'req' is true, a confirm dialog will pop-up asking if the player wants to install the part; when the player clicks OK, a few things are supposed to happen: the part is removed from the inventory; 'have' is changed to false; 'done' is changed to true; the part is removed from the inventory array. The only thing that does not seem to work is changing the object properties in the array.
<<for $i to 0; $i lt $genParts.length; $i++>>
	<<if $genParts[$i].req is true>>
		<<click $genParts[$i].name>>
			<<set $install to confirm('Install this part?')>>
				<<if $install is true>>
					<<set $genParts[$i].done to true>>
					<<set $genParts[$i].have to false>>
					<<set $partsInv.splice($partsInv.indexOf($genParts[$i]), 1)>>
				<<endif>>
		<</click>>
		<<print "<br>">>
	<<elseif $genParts[$i].have is true>>
		<<click $genParts[$i].name>>
			<<set alert('You don\'t have what you need for this.')>>
		<</click>>
		<<print "<br>">>
	<<endif>>
<</for>>

When I click OK to install a part, I get a "cannot set property of undefined" error, so it seems that the problem is in modifying the object in the array. What am I doing wrong?

Comments

  • The $i variable within the <<click>> macro's body is evaluated when the link is selected, so it will always be equal to the number of elements within the $genParts array. This is why you receive the error you do because there is no element with that index in the array.

    You need to change the <<click>> macro and its body into a string and concatenate the current value of $i to that string for each loop, then use a <<print>> macro to convert the string into code.

    Try something like the following, I added an extra $j variable which is in place of $i in a number of cases and I did that so I did not have to concatenate the current value of $i to the string more that once.
    <<for $i to 0; $i lt $genParts.length; $i++>>
    	<<if $genParts[$i].req>>
    		<<print "" + 
    		"<<click $genParts[$i].name>>" +
    			"<<set $j to " + $i + ">>" +
    			"<<set $install to confirm('Install this part? ' + $j)>>" +
    			"<<if $install>>" +
    				"<<set $genParts[$j].done to true>>" +
    				"<<set $genParts[$j].have to false>>" +
    				"<<set $partsInv.splice($partsInv.indexOf($genParts[$j]), 1)>>" +
    			"<<endif>>" +
    			"<<unset $j>>" +
    		"<</click>>">>
    		<<print "<br>">>
    	<<elseif $genParts[$i].have>>
    		<<click $genParts[$i].name>>
    			<<set alert('You don\'t have what you need for this.')>>
    		<</click>>
    		<<print "<br>">>
    	<<endif>>
    <</for>>
    
    note: you don't need to format the long string like I did, I inserted string concatenation, indents and line-breaks into the example so that it would be more readable.
  • Issue #1:
    You're not capturing the value of $i, so it may be used correctly within the <<click>>. Unless you capture the value of $i at the time each <<click>> is created for its use, they'll all receive the value $i has when they're invoked—meaning the value of $genParts.length, since that's how the loop left it (unless you changed it further before the player can use any of the <<click>> macros, in which case…). In either case, you're likely trying to use an out of bounds index.

    Traditionally, you'll want to <<print>> the macro in order to capture the correct value of $i. For example:
    <<print
    	  '<<click $genParts[$i].name>>'
    	+ '<<set $install to confirm("Install this part?")>>'
    	+ '<<if $install>>'
    	+ '<<set $genParts[' + $i + '].done to true>>'
    	+ '<<set $genParts[' + $i + '].have to false>>'
    	+ '<<set _partIdx to $partsInv.indexOf($genParts[' + $i + '])>>'
    	+ '<<if _partIdx gt -1>><<set $partsInv.splice(_partIdx, 1)>><</if>>'
    	+ '<</if>>'
    	+ '<</click>>'
    >>
    
    You, obviously, need to be mindful of your quotes when doing that. Also, you may note that I didn't bother capturing the value of $i within the <<click>> argument string, which is because you only need to capture the ones used within the body (inside) of the <<click>>.


    Issue #2:
    You're using the return value of $partsInv.indexOf(…) directly within $partsInv.splice(…), without checking to see if it actually found something—you should always check. <Array>.indexOf() returns -1 upon failure, in which case you're returning -1 to $partsInv.splice(…) as the index to remove—meaning that you're actually removing the last element within $partsInv every time you do. You should be doing something like the following:
    <<set _partIdx to $partsInv.indexOf($genParts[$i])>>
    <<if _partIdx gt -1>>
    	<<set $partsInv.splice(_partIdx, 1)>>
    <</if>>
    
    Or, better yet, if you've upgraded to SugarCube v2.5.0, simply use <Array>.delete():
    <<set $partsInv.delete($genParts[$i])>>
    
    Of course, neither of those will work if you're running into issue #3, however, they are what you should be doing, in general, since they won't erroneously pop elements off the end of the array if they can't find the element.


    (Possible) Issue #3:
    Without seeing more of your code I can't know for sure, however, it's possible that you're also running into variable store cloning. Each moment within the state history has its own, cloned, copy of the variable store. Because the variable store is cloned from one moment to the next, the "same" objects are not equal between moments—equivalent possibly, but not actually equal (i.e. they may have all the same properties and values, but they don't actually reference the same object). So, unless the contents of $partsInv and $genParts have had their contents synchronized the turn in which you're doing this, then the object at $genParts[$i] is never going to be found within $partsInv.


    Additional comments and suggestions:
    1. Unless you require its value within a future turn, I'd suggest using a temporary variable for your loop index (i.e. _i rather than $i). The same goes for any other temporary value which is only needed within its turn (e.g. maybe _install vs. $install).
    2. Comparing a boolean value to one of the boolean literals is pointless. Simply use the value in the conditional (e.g. <<if $genParts[$i].req>> and <<if $install>>).
    3. If you're only using $install in that one place, you don't even need a variable. Simply place the confirm within the <<if>> (e.g. <<if confirm(…)>>).
    4. You don't need to print "<br>". Simply use <br> (or a blank line for that matter).
    5. Don't mix macro ending styles. Use <</…>> or <<end…>>, not both.
    For example:
    <<for _i to 0; _i lt $genParts.length; _i++>>
    	<<if $genParts[_i].req>>
    		<<print
    			  '<<click $genParts[_i].name>>'
    			+ '<<set $install to confirm("Install this part?")>>'
    			+ '<<if $install>>'
    			+ '<<set $genParts[' + _i + '].done to true>>'
    			+ '<<set $genParts[' + _i + '].have to false>>'
    			+ '<<set _partIdx to $partsInv.indexOf($genParts[' + _i + '])>>'
    			+ '<<if _partIdx gt -1>><<set $partsInv.splice(_partIdx, 1)>><</if>>'
    			+ '<</if>>'
    			+ '<</click>>'
    		>>
    		<br>
    	<<elseif $genParts[_i].have>>
    		<<click $genParts[_i].name>>
    			<<set alert("You don't have what you need for this.")>>
    		<</click>>
    		<br>
    	<</if>>
    <</for>>
    
  • Sorry for the super late reply, but this is all very helpful, thank you! I'm very new to code and I'm still learning best practices, so I really appreciate those tips. And somehow I missed <Array>.delete() entirely while looking for ways to do that, so thank you for pointing that out, whoops.

    It sounds like issue #3 is the root of the problem, but I have a few things I'm not quite clear on, if you don't mind explaining further.

    Functionally, what is the difference between a temporary and regular value (_i vs $i)? This is the first I've seen of that syntax so I'm not familiar with it. So, does this mean that the variable is deleted after the <<for>> loop?

    But most importantly: it sounds like synchronizing those two arrays is the most important part, but I'm not sure how I'd go about doing that. How does one synchronize arrays?
  • […] And somehow I missed <Array>.delete() entirely while looking for ways to do that, so thank you for pointing that out, whoops.
    It's new in v2.5.0, so if you checked the docs prior to that release, then it simply wasn't there to be found.

    It sounds like issue #3 is the root of the problem, but I have a few things I'm not quite clear on, if you don't mind explaining further.
    No. Issue #1 is the cause of your reported problem (i.e. "cannot set property of undefined"), as I noted—though issue #2 wasn't helping matters.

    That your code may also be impacted by what I outlined as "possible issue #3" is a separate can of worms. We'd need to know the particulars of how and when each array is constructed to be able to definitively say whether or not you're running afoul of it. It's not a given.

    Functionally, what is the difference between a temporary and regular value (_i vs $i)? This is the first I've seen of that syntax so I'm not familiar with it. So, does this mean that the variable is deleted after the <<for>> loop?
    Story variables, those starting with the '$' sigil, are part of the story history and are permanent (i.e. once set, they only change if your code changes them).

    Temporary variables, those starting with the '_' sigil, are not part of the story history and are automatically discarded at the start of passage navigation (i.e. going from one passage to another; simply displaying a passage via <<display>> does not count as navigation).

    But most importantly: it sounds like synchronizing those two arrays is the most important part, but I'm not sure how I'd go about doing that. How does one synchronize arrays?
    You may already be doing so. Again, we'd need to know the particulars of how and when you are constructing each array. That said, and in basic, you'd probably need to create one of the arrays from the other within the passage that the above code is from (e.g. at the start).

    You stated that the $genParts array is created at the start of the install passage. You did not note, however, if it was created from the $partsInv array and/or if the passage which contains the code shown above is the install passage or a subsequent one.
Sign In or Register to comment.