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.

Sign up for GitHub

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

Jump to bottom

Weather - Properly sync overcast value #5688

Merged
merged 1 commit into from
Oct 27, 2017
Merged

Weather - Properly sync overcast value #5688

merged 1 commit into from
Oct 27, 2017

Conversation

ulteq
Copy link
Contributor

@ulteq ulteq commented Oct 26, 2017

  • Makes sure that all clients share the same (numeric) overcast value
  • Only affects the barometric pressure calculation
  • Does not affect the actual weather on the clients

Avoids barometric pressure synchronization issues between clients due to overcast drift.

Reference: https://cuel.github.io/

@ulteq
Weather - Properly sync overcast value
4d16651 
* Makes sure that all clients share the same (numeric) overcast value
* Only affects the barometric pressure calculation
* Does not affect the actual weather on the clients
@ulteq ulteq added kind/bug-fix Release Notes: **FIXED:** status/WIP labels Oct 26, 2017
@ulteq ulteq added this to the 3.12.0 milestone Oct 26, 2017
@ulteq ulteq removed the status/WIP label Oct 27, 2017
jonpas
jonpas reviewed Oct 27, 2017
View reviewed changes
addons/weather/XEH_postInit.sqf
@@ -11,13 +11,14 @@ GVAR(wind_period_start_time) = CBA_missionTime;
if (!isServer) then {
"ACE_MISC_PARAMS" addPublicVariableEventHandler {
TRACE_1("MISC PARAMS PVEH",ACE_MISC_PARAMS);
GVAR(currentOvercast) = (ACE_MISC_PARAMS select 0);
Copy link
Member

Choose a reason for hiding this comment

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

I don't know why there are () on the right side in the rest of the params, but they are really pointless.

Copy link
Contributor Author

@ulteq ulteq Oct 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

We can fix it in the following cleanup PR. This part of the code is going to be removed anyways.

Copy link
Contributor

@PabstMirror PabstMirror Oct 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This only runs on non servers
So GVAR(currentOvercast) is always 0 on server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The server never uses:
GVAR(currentTemperature), GVAR(currentHumidity) and GVAR(currentOvercast)

@jonpas jonpas added kind/enhancement Release Notes: **IMPROVED:** and removed kind/bug-fix Release Notes: **FIXED:** labels Oct 27, 2017
@ulteq ulteq merged commit cda060d into master Oct 27, 2017
@ulteq ulteq deleted the weather-overcast-value-synchronization branch October 27, 2017 10:39
@PabstMirror
Copy link
Contributor

imho, instead of syncing with a pub variable "event" in ACE_MISC_PARAMS
we should just do missionNamespace setVariable ["ace_weather_overcast", overcast, true]
skip the need for any sqf to run on the clients

@ulteq
Copy link
Contributor Author

ulteq commented Oct 29, 2017 •
edited
Loading

Yes, I'm going to remove those public variable event handlers entirely in the following cleanup pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PabstMirror PabstMirror PabstMirror left review comments

@jonpas jonpas jonpas approved these changes

Assignees

@jonpas jonpas

Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Milestone
3.12.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ulteq @PabstMirror @jonpas