Announcement: Be excellent to each other.


Caravel Forum : DROD Boards : Bugs : Combining and splitting yellow doors via build commands (What a headache!)
12
Page 3 of 3
New Topic New Poll Post Reply
Poster Message
skell
Level: Legendary Smitemaster
Avatar
Rank Points: 3500
Registered: 12-28-2004
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+2)  
So apparently the splitting logic was indeed there, though I reimplemented it myself because I had broken tests and thought it isn't there.

Anyway, built doors will now merge to avoid duplicate connections to the same orb/plate, and any conflicting actions will be resolved to toggle.

PR

This is the last thing I had in store for 5.1.1, time to try to get the Linux build to, erm, build.

____________________________
My website | Facebook | Twitter
10-30-2020 at 07:13 PM
View Profile Send Private Message to User Send Email to User Visit Homepage Show all user's posts High Scores This architect's holds Quote Reply
kieranmillar
Level: Smitemaster
Rank Points: 2272
Registered: 07-11-2014
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (0)  
Just tested this in 5.1.1.alpha.2020-10-29 and I don't think it's working as intended.

Scenario:

There are two doors, both controlled by the same orb. A build command merges these two doors into one. End result: The orb maintains two different connections to the same door, and its effects stack. So for example, if the orb toggled both doors, now when you strike the orb, it will toggle the door twice, so effectively nothing happens.
11-01-2020 at 10:47 AM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
skell
Level: Legendary Smitemaster
Avatar
Rank Points: 3500
Registered: 12-28-2004
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (0)  
The status says it's still waiting for merge so I don't think it made it into the build yet

____________________________
My website | Facebook | Twitter
11-01-2020 at 11:40 AM
View Profile Send Private Message to User Send Email to User Visit Homepage Show all user's posts High Scores This architect's holds Quote Reply
kieranmillar
Level: Smitemaster
Rank Points: 2272
Registered: 07-11-2014
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (0)  
Oh yeah, of course, I should have guessed a change made on the 30th would be in the 29th alpha build. :facepalm:
11-01-2020 at 11:50 AM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
TFMurphy
Level: Smitemaster
Rank Points: 3065
Registered: 06-11-2007
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+2)  
Couple of observations about the algorithm you've used to merge the doors:


1. As it stands now, the code will always run a merge if at least one agent is found. That's unnecessary. It'd be better to keep a running count of all agents found pointing to the doorCoords, and do the merge only if it's greater than 1.


2. You don't need to collect boolean information about bHasOpen/bHasClose/bHasToggle. You can start with 'OrbAgentType targetAction = OA_NULL' and just change that as you iterate over each agent (since we've chosen a solution where order doesn't matter). The code for that would look something like this:
   agentsFound++;

   if (pAgentData->action == OA_NULL)
      continue;  // Really shouldn't happen... might be worth an assert as well?

   if (targetAction == OA_NULL)
      targetAction = pAgentData->action;
   else if (targetAction != pAgentData->action)
      targetAction = OA_TOGGLE;


Feels like there could be a way to reduce the routine to one loop through the agents (followed by an editing of the first Agent found). The first iteration could also be ended early if targetAction has become OA_TOGGLE and agentsFound >= 2.

But both of those suggestions are probably a bit more involved for not much gain. Definitely think avoiding a second iteration if there's only one agent at all is worthwhile though.


Other than that, the whole thing looks a lot neater than I probably would've done it -- was always more comfortable editing old code than creating new. Nice work!
11-01-2020 at 02:11 PM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
kieranmillar
Level: Smitemaster
Rank Points: 2272
Registered: 07-11-2014
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (0)  
kieranmillar wrote:
Just tested this in 5.1.1.alpha.2020-10-29 and I don't think it's working as intended.

Scenario:

There are two doors, both controlled by the same orb. A build command merges these two doors into one. End result: The orb maintains two different connections to the same door, and its effects stack. So for example, if the orb toggled both doors, now when you strike the orb, it will toggle the door twice, so effectively nothing happens.
This is still happening in 5.1.1.alpha.2020-11-04 for what it's worth, I'm not seeing any changes. Maybe it's not in this alpha either?
11-07-2020 at 11:25 AM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
skell
Level: Legendary Smitemaster
Avatar
Rank Points: 3500
Registered: 12-28-2004
IP: Logged

File: BUG REPORT Merging yellow doors.hold (1.1 KB)
Downloaded 2 times.
License: Public Domain
icon Re: Combining and splitting yellow doors via build commands (+1)  
@kieranmillar - Seems to work fine for me. I've downloaded 5.1.1.alpha.2020-11-04 from this post and tested the attached hold, and didn't get multiple connections. Can you please share the hold you used for testing?

____________________________
My website | Facebook | Twitter
11-08-2020 at 10:34 AM
View Profile Send Private Message to User Send Email to User Visit Homepage Show all user's posts High Scores This architect's holds Quote Reply
kieranmillar
Level: Smitemaster
Rank Points: 2272
Registered: 07-11-2014
IP: Logged

File: BUG REPORT Multi orb connections.hold (1 KB)
Downloaded 2 times.
License: Public Domain
icon Re: Combining and splitting yellow doors via build commands (+1)  
See attached hold. Your hold works for me, but this hold does not.
11-08-2020 at 11:02 AM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
TFMurphy
Level: Smitemaster
Rank Points: 3065
Registered: 06-11-2007
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+3)  
Oh my, that's a pretty fantastic bug. I mean, not the bug itself, but just the fact that skell's test cast just *happens* to evade it.

It's due to the new code in MergeYellowDoorConnectionsInArea. It's supposed to test all coordinates from (wX,wY)-(endX,endY).

Unfortunately, it's currently testing (wY,wX)-(endX,endY).

So a door built at (12,12) works fine. But one built from (16,11)-(18,13) ends up testing (11,16)-(18,16), and so ends up not testing any of the doors in that room at all. Simple mistake, but it's amazing that the original test case just happens to pick one of the few ways it'd actually work properly.
11-08-2020 at 03:37 PM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
skell
Level: Legendary Smitemaster
Avatar
Rank Points: 3500
Registered: 12-28-2004
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+2)  
I am not even mad that's amazing

Edit: PR

@TFM: I also included your optimization suggestions. There was another issue - it would add the same orb multiple times to the vector and merge the orb once for each agent.

____________________________
My website | Facebook | Twitter

[Last edited by skell at 11-08-2020 04:27 PM]
11-08-2020 at 03:53 PM
View Profile Send Private Message to User Send Email to User Visit Homepage Show all user's posts High Scores This architect's holds Quote Reply
TFMurphy
Level: Smitemaster
Rank Points: 3065
Registered: 06-11-2007
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+2)  
skell wrote:
@TFM: I also included your optimization suggestions. There was another issue - it would add the same orb multiple times to the vector and merge the orb once for each agent.
Hmm... well, the main optimization issue you're running into is that you're iterating over the agents far too many times. EDIT: To be clear here, I'd just been concentrating on MergeOrbConnections when I was looking at the code originally, so I thought you were looking at each orb a maximum of twice. I hadn't realised there was already another iteration in GetOrbConnections, so I'm addressing that now in this post.

The code looks like it's doing something like this:

* GetOrbConnections: Iterate through all orbs in the room. Iterate through all agents in each orb. If agent connects to merged door, mark orb for merging. (With the PR, this now only marks an orb if 2 or more agents connect to merged door.)
* MergeOrbConnections Part 1: Iterate through all agents in a marked orb. Collect information about each action of the agent if it points to merged door. If at least one action is found, move to Part 2.
* MergeOrbConnections Part 2: Iterate through all agents in a marked orb. If it doesn't point to the merged door, ignore it. If it's the first action found that does so, edit the action to whatever was decided in Part 1; if it's not the first action, delete it.

So each orb is being iterated through at least once, and merged orbs are being looked through up to three times.

Instead, you could drop GetOrbConnections, and do something like this:

* MergeOrbConnections Part 1: Iterate through all agents in an orb (marked or not). Collect information about each action of the agent if it points to merged door. Ignore actions that don't point to merged door. If it's the first action found, keep track of it. If it's not the first action but still must be merged, delete it.
* MergeOrbConnections Part 2: Edit the first action found with the combined action we calculated in Part 1.

This way, each orb is iterated over once only, merged actions are removed immediately, and orbs that have no actions that need to be merged are already ignored.

[Last edited by TFMurphy at 11-08-2020 05:25 PM]
11-08-2020 at 04:57 PM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
skell
Level: Legendary Smitemaster
Avatar
Rank Points: 3500
Registered: 12-28-2004
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+2)  
I'd rather keep it as is - there are no cases where it would make any discernable difference and changing it would likely reduce reafability.

____________________________
My website | Facebook | Twitter
11-08-2020 at 05:17 PM
View Profile Send Private Message to User Send Email to User Visit Homepage Show all user's posts High Scores This architect's holds Quote Reply
kieranmillar
Level: Smitemaster
Rank Points: 2272
Registered: 07-11-2014
IP: Logged
icon Re: Combining and splitting yellow doors via build commands (+1)  
Can confirm that this is now all working in 5.1.1.beta.2020-11-12. When different doors with commands from the same source are merged into one and they had different effects on the doors, they turn into toggles.
11-12-2020 at 08:23 PM
View Profile Send Private Message to User Show all user's posts High Scores This architect's holds Quote Reply
12
Page 3 of 3
New Topic New Poll Post Reply
Caravel Forum : DROD Boards : Bugs : Combining and splitting yellow doors via build commands (What a headache!)
Surf To:


Forum Rules:
Can I post a new topic? No
Can I reply? No
Can I read? Yes
HTML Enabled? No
UBBC Enabled? Yes
Words Filter Enable? No

Contact Us | CaravelGames.com

Powered by: tForum tForumHacks Edition b0.98.8
Originally created by Toan Huynh (Copyright © 2000)
Enhanced by the tForumHacks team and the Caravel team.