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

Initial support for MUME's room id #308

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

Initial support for MUME's room id #308

wants to merge 16 commits into from

Conversation

cosmos72
Copy link
Contributor

@cosmos72 cosmos72 commented Apr 3, 2023

Parses MUME's room id into a newly created class RoomServerId,
and also adds a RoomServerId field to relevant classes: Room and ParseEvent.

Definitely NOT complete, but it's a first step.

The path machine code definitely needs review from someone who knows it better than me.

@nschimme nschimme marked this pull request as draft April 30, 2023 14:36
@Sebster7
Copy link

Sebster7 commented Sep 5, 2023

I've been following this merge request, but there's been no activity. What are the next steps here @nschimme ? I feel support for MUME's room ids is the most important feature in the potential pipeline for MMapper right now...

@cosmos72
Copy link
Contributor Author

I'll try to answer while waiting for @nschimme take on this:
This patch only contains the bare minimum needed to receive, extract and pass around MUME room ids.

The next step is extending path machine algorithms to add MUME room ids as a very high priority/confidence source of location info.
Unluckily, the path machine is definitely complicated and I currently don't have the time to study it - someone else (@nschimme ?) will be needed to extend it.

@Sebster7
Copy link

Sebster7 commented Sep 18, 2023

Thanks for your reply @cosmos72. I'd say that it would be good to get in the parsing and saving of the current room id to the map (where the current location is known with a high degree of certainty using the existing algorithm), and ignore everything else GMCP-related (as that may - or should - yet change), and including the path machine stuff. This way, people can start to get their maps updated (as the default map having this is sort of a blocker on #306 ).

@cosmos72
Copy link
Contributor Author

cosmos72 commented Sep 20, 2023

Hi @Sebster7,
that was exactly my thought when I started adding support for room ids.

Although, it turned out that this means reconciling room ids with the other mechanisms used by path machine for recognizing and identifying rooms.

So you still have to modify the path machine, although the changes are probably simpler: after all, the intention is to only WRITE room ids into room, not to READ them back and use them as an additional source of truth inside path machine algorithm.

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