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

Improve layer information extraction #363

Merged

Conversation

denizetkar
Copy link
Contributor

@denizetkar denizetkar commented Jul 7, 2024

Description

  • SquadServer regularly invokes updateLayerInformation to update the current and next layer variables, namely this.currentLayer and this.nextLayer.
  • This is fetched from Layers class, getLayerByName method, which filters the pulled layers this.layers by the name field.
  • The layers are fetched from https://raw.githubusercontent.com/Squad-Wiki/squad-wiki-pipeline-map-data/master/completed_output/_Current%20Version/finished.json
    • I'm actually a bit concerned that this "current version" is outdated with respect to the latest Squad version 8.0.
  • However, the layer name for filtering is fetched from SquadRcon class, getCurrentMap method which uses the RCON command ShowCurrentMap. This command returns an answer like Current level is Jensen's Range, layer is JensensRange_USA-RGF, factions USA RGF. The problem here is 2-fold:
    • The regex used to extract the level and the layer names is broken: ^Current level is (.*), layer is (.*) Because the 2nd regex group actually matches a string like JensensRange_USA-RGF, factions USA RGF
    • Even if we correctly extract the "layer name", it actually corresponds to the layerid field in the pulled layers

What this PR does

Solves those 2 problems by:

  • Updating the regexes to extract the level and the layer ids from the RCON commands ShowCurrentMap and ShowNextMap as: ^Current level is ([^,]*), layer is ([^,]*)
  • Changing the matching logic to use the layerid field instead of name

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be an accidental edit but other than that looks good to me. Going to test it soon.

Copy link
Collaborator

@werewolfboy13 werewolfboy13 left a comment

Choose a reason for hiding this comment

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

Changes are required.

squad-server/plugins/discord-killfeed.js Outdated Show resolved Hide resolved
@denizetkar denizetkar force-pushed the fix/server-current-layer branch from d1ceee1 to 92e9e4a Compare July 9, 2024 19:39
@Ulibos
Copy link
Contributor

Ulibos commented Jul 11, 2024

I have submitted a PR that addresses duplicate Victim field that you reported: #364

@denizetkar
Copy link
Contributor Author

Hi @Ulibos,

Any updates on testing the PR?

@Ulibos
Copy link
Contributor

Ulibos commented Jul 13, 2024

We are using a plugin that queries data from layers infrequently so I can't vouch for a full comprehensive test but so far I haven't seen a single failure on this part of the code.

@denizetkar
Copy link
Contributor Author

Should we merge it then?

@Ulibos
Copy link
Contributor

Ulibos commented Jul 17, 2024

I'm all for merge but I'm not the one calling the shots.

@werewolfboy13
Copy link
Collaborator

There is another merge about to happen soon that will require rewrites again. Merges are on hold until then.

@werewolfboy13
Copy link
Collaborator

Force pushing. Mobile UI is preventing conversation resolution.

@werewolfboy13 werewolfboy13 added core bug Bug related to the core SquadJS API patch Patch Change labels Aug 10, 2024
@werewolfboy13 werewolfboy13 merged commit f99aacf into Team-Silver-Sphere:master Aug 10, 2024
@Thomas-Smyth Thomas-Smyth changed the title fix: update the server layer extraction Improve layer information extraction Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core bug Bug related to the core SquadJS API patch Patch Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants