Skip to content

Conversation

@kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Jan 18, 2020

fixes #538
I have explained the math behind my approach in #576 (comment) . Please read it.
as suggested by @aubergine10 in #576 (comment) I provided option to "buff the clickability of segments based on their length". The option can be configured at compile time.

fixes #594
To fix this: when mouse ray is not valid I set HoveredSegmentId and HoveredSegmentId to zero

EDIT: fixes #616
I added an exception for when control key is down.

}

return HoveredNodeId != 0 || HoveredSegmentId != 0;
if (HoveredNodeId != 0) {
Copy link
Collaborator Author

@kianzarrin kianzarrin Jan 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argument can be made that his should be done at line 877 after we determine HoveredNodeID. That approach works for all cases except for level crossings. I found no way to get the following function to find level crossing nodes:
RayCast(segmentInput, out RaycastOutput segmentOutput)
Therefore I decided to call GetHoveredSegmentFromNode() here.

The code can take multiple paths. the most complicated one is this:

  1. We determine the HoveredSegmentID.
  2. We determine HoveredNodeID by calculating shortest distance to the segment start or end node.
  3. We fix HoveredSegmentID again by finding the segment which makes the minimum angle with the mouse-to-node vector.

it may sound confusing but thats the only way to support level crossings.

/// returns the node segment that is closest to the mouse pointer based on angle.
/// </summary>
internal ushort GetHoveredSegmentFromNode() {
bool considerSegmentLenght = true;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better place for this hard coded configuration option?

Copy link
Collaborator

@kvakvs kvakvs Jan 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a constant somewhere in the same class, or Constants.cs module (we have one or a few)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@kianzarrin
Copy link
Collaborator Author

I tested this with bridges, tunnels, metro, level crossings, and of course normal junctions.


protected override ushort HoveredNodeId {
get {
bool ctrlDown = Input.GetKey(KeyCode.LeftControl) || Input.GetKey(KeyCode.RightControl);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes #616

/// input order does not matter.
/// The return value is between 0 to 180.
/// </summary>
private static float GetAgnele(Vector3 v1, Vector3 v2) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming: GetAngle

Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good

@krzychu124 krzychu124 merged commit 447f957 into CitiesSkylinesMods:master Jan 20, 2020
@originalfoo originalfoo added this to the 11.0 milestone Jan 20, 2020
@originalfoo originalfoo added LANE ROUTING Feature: Lane arrows / connectors UI User interface updates Usability Make mod easier to use labels Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LANE ROUTING Feature: Lane arrows / connectors UI User interface updates Usability Make mod easier to use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lane arrow tool may selects wrong node when holding ctrol The Show error dialog panel can enter a vicious circle

4 participants