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

Added name to map markers #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Grimbyy
Copy link

@Grimbyy Grimbyy commented Apr 23, 2023

Adds name to Note struct

New rust update allows you to have up to 5 labels now, with colors, symbols and names.

image

@s8wa2
Copy link

s8wa2 commented Apr 12, 2024

Looks like there may be some confusion.

Map markers (notes) are not returned in AppMap (from getMap request), but rather AppTeamInfo.Note (from getTeamInfo). You added it to the wrong message struct.

I took a look at both aux01 and current server code. Monument type has not changed. However note now has the following properties:

public int type;
public float x;
public float y;
public int icon;
public int colourIndex;
public string label;

I would recommend adding these fields to AppTeamInfo.Note

message Note {
	required int32 type = 2;
	required float x = 3;
	required float y = 4;
+	required int32 icon = 5;
+	required int32 colourIndex = 6;
+	optional string label = 7;
}

And removing this from AppMap.Monument

message Monument {
	required string token = 1;
	required float x = 2;
	required float y = 3;
-	required float name = 7;
}

Great work getting the right field number for label 7 from 58 >> 3

The reason it's optional is because it only writes if string is not null.

if (instance.label != null)
        {
          stream.WriteByte((byte) 58);
          ProtocolParser.WriteString(stream, instance.label);
        }

Compare that to a required string like member.name:

if (instance.name == null)
          throw new ArgumentNullException("name", "Required by proto specification.");

At least from my understanding. Not 100% certain on how liam made the original proto schema.

@jamezrin
Copy link

Map markers (notes) are not returned in AppMap (from getMap request), but rather AppTeamInfo.Note (from getInfo). You added it to the wrong message struct.

Notes are actually returned from the getTeamInfo request. The getInfo request returns info about the server

@s8wa2
Copy link

s8wa2 commented Apr 13, 2024

Thanks! Updated original comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants