KillBait / AutoDrive_Course_Editor

A java based course editor for the AutoDrive mod, works with both the FS19 + FS22 versions of the mod
73 stars 13 forks source link

[Feature request] Highlight invalid nodes #14

Closed Laku83 closed 2 years ago

Laku83 commented 2 years ago

Hi @KillBait, this is not really an issue but more of a usefull feature you could add. When creating the default tracks on a new FS22 game, Autodrive didn't connect correctly the path at each crossing, so now I'm doing it manually with the editor. Would be usefull if the editor could assign a different colors to nodes with same coordinates that are not connected (whith a tollerance). Or maybe a yellow triangle near the nodes to attract the attention of the user and possibly correct the issue.

More andvanced would be an auto correction algorithm. :) But I'm not

https://user-images.githubusercontent.com/3968257/147572181-9a832d17-1096-4597-990b-91b06d91582f.mp4

KillBait commented 2 years ago

I have thought about adding such a thing in the last few days, i was leaning more towards the notification icon like you suggested, there may be a way to auto merge overlapping nodes, but i would have to think about how that would work.

The only issue i can see come down to how the editor stores the node data, the way the editor works ia to check the area around each node it has to scan the entire network, this isn't such a big issue with low node counts, but if you have for example 5000 nodes and each check is 5000 times per node (makes 25 million checks),

I will look into it, but how to implement it is the bigger issue

Thanks for the suggestion.

Laku83 commented 2 years ago

As for my understanding, each node has inputs and outputs, I think if only checking for this information: if node has only 1 input or only 1 output then is an end point so it's a warning. In this way if you have 5000 nodes you have to check only 5000 times. I haven't read the code, but I assume when you read all the node from the config you could check for this as well and assign a flag to that node. Following this: knowing wich are the problematic nodes you could further check between them if they are duplicates and do other stuff to correct the error. Knowing the total number it's also possible to warn the user if they are to many and can take much time and eventually stop the procedure, or do this operation only on manually selected areas to allow resolution of messed up network.

I've also written an issue in the AutoDrive github so hopefully Stephan will solve this on is end too.

Thank you for your great editor.

KillBait commented 2 years ago

If the check was to take multiple seconds, how would that be handled?, scan the network on the config load and have a possible multiple second wait for it to finish, or run it in the background and have it update as is scans?

My preference would be the latter.

Thinking about the merging, i'm fairly sure i could pull that off without a lot of issues, just store all the incoming/outgoing connections, remove all but one node and just combine all the connections into that single node.

Laku83 commented 2 years ago

I was thinking after populating these values in the map node:

(GameXML.java)

for (int i=0; i<ids.length; i++) {
 MapNode mapNode = nodes.get(i);
      String[] outNodes = outValueArrays[i].split(",");
      for (String outNode : outNodes) {
        if (Integer.parseInt(outNode) != -1) {
            mapNode.outgoing.add(nodes.get(Integer.parseInt(outNode) - 1));
       }
    }
}
for (int i=0; i<ids.length; i++) {
    MapNode mapNode = nodes.get(i);
     String[] incomingNodes = incomingValueArrays[i].split(",");
     for (String incomingNode : incomingNodes) {
         if (Integer.parseInt(incomingNode) != -1) {
            mapNode.incoming.add(nodes.get(Integer.parseInt(incomingNode)-1));
     }
   }
}

A check could be added to see if the node is a dead end:

for (int i=0; i<ids.length;i++) {
    MapNode mapNode = nodes.get(i);
    if (mapNode.incoming.length == 0 || mapNode.outgoing == 0) {
        mapNode.isDeadEnd = true;
   }
}

And add a flag in the MapNode to store the information:

(MapNode.java)

this.isSelected = isSelected;
this.isControlNode = isControlNode;
**this.isDeadEnd = false;**            // Default is a normal node

Then use this information when rendering the nodes. But here I don't know how the GUI works.

For the merging I think what you wrote is the correct way. To not lock the interface in long processes the user could select an area and excetute a command only on the selected nodes. I'm thinking some times deadends nodes could be wanted (double direction routes, or an open branch leaved for future network expansions) in this case the user know what he wants.

I'm sorry I don't understand a lot of java otherwise I would pull a commit myself, hope to be of help.

KillBait commented 2 years ago

this is a test at the moment, its just a menu triggered scan but it marks the overlapping nodes.

Screenshot 2021-12-28 173823

I did try it on a config i know has around 15,000 nodes and the delay is about 2 seconds or so to scan the whole network

Laku83 commented 2 years ago

You are great!

KillBait commented 2 years ago

I've updated the beta branch with the (almost, nothing config breaking) bug free version of the code, the editor will scan the network in the background after it is loaded.

Sorry for the delay. just started a new job, so i have had almost no time to work on it until today.

Laku83 commented 2 years ago

I've just tested it: it showed the overlapping nodes in 0ms (it was instantaneous) and I found a bunch of nodes overlapping after all the manual checking, so I can confirm this feature is very usefull. But I don't really know how many nodes I have. Where do I see this information? I've only the automatically created network plus a little amount that I've created myself in game. But overall is very smooth.

Thanks for your awesome work!

KillBait commented 2 years ago

it outputs very basic info to the log at the moment, you should see something like this

Screenshot 2022-01-01 114447 - that was generated from Elmcreek and AD v2.0.0.0

Laku83 commented 2 years ago

In my log is jumping a line:

[INFO ] 2022-01-01 16:42:28.882 [SwingWorker-pool-3-thread-1] AutoDriveEditor - Starting Background Scan for Overlapping Nodes
[INFO ] 2022-01-01 16:42:29.023 [SwingWorker-pool-3-thread-1] AutoDriveEditor - Finished Background Scan

maybe it's too fast :)

KillBait commented 2 years ago

if you run the editor with the -DEBUG ( java -jar -Xms1024m -Xmx1024m .\AutoDriveEditor.Jar -DEBUG ) you will find the debug menu, select 'Show Profile Times' and load a config, look in the console or log and you should see the time

Laku83 commented 2 years ago

Ok i found it:

[INFO ] 2022-01-01 17:24:18.050 [SwingWorker-pool-3-thread-4] AutoDriveEditor - Starting Background Scan for Overlapping Nodes
[INFO ] 2022-01-01 17:24:18.150 [SwingWorker-pool-3-thread-4] AutoDriveEditor - Roadmap nodes = 5910 --- Found 2 nodes overlapping
[INFO ] 2022-01-01 17:24:18.150 [SwingWorker-pool-3-thread-4] AutoDriveEditor - Finished Background Scan in 0.1s

But it seems it logs the node number only when it find an overlapping node. To show this information I've added an overlapped node and then triggered the scan via the menu. It also seems that the tollerance is really "strict" (low range). How many meters it check for overlapping? I had to move the second node almost perfectly on the other one to make it recognize the overlap. Is this wanted?

KillBait commented 2 years ago

the default check distance around each node is very small, 0.05 meters, it will be configurable, i just didn't get around to adding the dialog to change it. Same applies to the nodes info, i was going to add a dialog just to show the info after a scan, again, havn't got around to it yet. The main priority was to get it working and test it

I'm currently pulling hair out getting the merge node to work properly, it proving way more complex than it looks :) , i will prob have a break from it and add the dialog and area options

KillBait commented 2 years ago

I just pushed a new commit to beta branch addressing the issues you pointed out.

A manual scan will ask for a overlap distance before running, the output of the scan is put into the text area at the bottom of the editor.

I also included the merging nodes code, but you will have to run the editor with -EXPERIMENTAL to use it. It's not 100% accurate as there some weird edge cases i still have to account for ( figuring one out took me ~7 hours! ) but it works pretty good as-is. My advice is run the merging on a backup config in case something goes pear shaped.

Laku83 commented 2 years ago

Hi KillBait, I just tested the new beta, for this try I've generated a completely new network via the auto generation of autodrive in erlengrat; this network is completely messed up. This time the editor took some seconds to process all the overlapping nodes. I've tried also the merging feature: it works 99% of the times, sometimes it creates strange paths back and forward. Also I noticed that if I add manually a node overlapping an existing one, this isn't considered as overlapping. I haven't tried to save and scan again the map. I'm attaching the original version of the map and the log if that could be useful. (change extension to the autodrive config) See in the upper left corner: autodrive pratically generated a double path natively overlapped. In the most upper left crossroad I found the weird back and forward path I was talking about. autoDriveEditor.log AutoDrive_config.xml

Edit: So I can confirm that manually added nodes are not considered overlapped:

KillBait commented 2 years ago

Thanks for feedback 👍

You have to remember it also checks the the nodes depth also, if you have two nodes that are in the same place and they don't merge, it may well be the depth is outside the scan range. If that is not the case, its another edge case to track down

This is related, I'm working on two more different icons to show on the scan alongside the overlapping nodes, one for where two nodes are overlapping but outside the scan range and the other is one where nodes are placed in editor with default Y level of -1.

[ EDIT ] i forgot to mention since the commit i have fixed one edge case in the merging code

KillBait commented 2 years ago

This goes alongside with the manual node location edit and depth alignment (like horizontal/vertical) i'm adding also