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

Ensure defaults set correctly - fix regressions #1464

Merged
merged 3 commits into from
Mar 13, 2022
Merged

Conversation

originalfoo
Copy link
Member

@originalfoo originalfoo commented Mar 12, 2022

Kian noticed some issues in earlier PR regarding mod option defaults.

#1455 (comment)

Notably:

  • Individual driving style used to be on by default
  • buses may ignore lane arrows used to be off by default

This PR fixes the first (driving styles) but leaves the second (bus ingore lane arrows) as defaulting to on - currently. Should I change bus ignore arrows back to off by default, or should it be on by default? It's trivial to change either way.

I also explicitly defined several false defaults in the options manager.

@originalfoo originalfoo added Settings Road config, mod options, config xml regression old bug comes back or a new bug introduced in code that used to work. labels Mar 12, 2022
@originalfoo originalfoo added this to the 11.6.5.1 milestone Mar 12, 2022
@originalfoo originalfoo self-assigned this Mar 12, 2022
These would get overwritten during deserialization so they had
no effect anyway. Removing to clarify that.
Comment on lines -41 to -42
public static bool individualDrivingStyle = true;
public static int recklessDrivers = 3;
Copy link
Member Author

@originalfoo originalfoo Mar 12, 2022

Choose a reason for hiding this comment

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

These default vals would get overwritten during deserialization (OptionsManager.LoadData()) so removed them to clarify they had no effect here in Options.cs.

EDIT: The fields are still there, I just removed their default vals. Embedded diff doesn't make that clear (at least in github dark theme).

image

@originalfoo originalfoo merged commit f264a2b into master Mar 13, 2022
@originalfoo originalfoo deleted the fix-defaults branch March 13, 2022 02:35
@krzychu124
Copy link
Member

@aubergine10 is this a result of recent changes?

Warning 3.9198840: Skipping invalid value 1 for vehicle restrictions aggression
   at CSUtil.Commons.Log.LogToFile(System.String log, LogLevel level)
   at CSUtil.Commons.Log.Warning(System.String s)
   at TrafficManager.Manager.Impl.OptionsManager.LoadData(System.Byte[] data)
   at TrafficManager.Lifecycle.SerializableDataExtension.Load()
   at TrafficManager.Lifecycle.TMPELifecycle.Awake()
   at UnityEngine.GameObject.Internal_AddComponentWithType(System.Type )
   at UnityEngine.GameObject.AddComponent(System.Type componentType)
   at UnityEngine.GameObject..ctor(System.String name, System.Type[] components)
   at TrafficManager.Lifecycle.TMPELifecycle.StartMod()
   at TrafficManager.Lifecycle.TrafficManagerMod.OnEnabled()
   at System.Reflection.MonoMethod.InternalInvoke(System.Object , System.Object[] , System.Exception ByRef )
   at System.Reflection.MonoMethod.Invoke(System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(System.Object obj, System.Object[] parameters)
   at ColossalFramework.Plugins.PluginManager.AddPlugins(System.Collections.Generic.Dictionary`2 plugins)
   at ColossalFramework.Plugins.PluginManager.LoadPluginAtPath(System.String path, Boolean builtin, PublishedFileId id)
   at ColossalFramework.Plugins.PluginManager.OnPluginAdded(System.String path)
   at ColossalFramework.Plugins.PluginManager+<>c__DisplayClass20.<OnFileWatcherEventChanged>b__1a()
   at ColossalFramework.Threading.Dispatcher+<>c__DisplayClass4.<CreateSafeAction>b__3()
   at ColossalFramework.Threading.Task`1+<>c__DisplayClassa[[ColossalFramework.Threading.Task+VoidTask, ColossalManaged, Version=0.3.0.0, Culture=neutral, PublicKeyToken=null]].<.ctor>b__8(ColossalFramework.Threading.Task t)
   at ColossalFramework.Threading.Task`1[[ColossalFramework.Threading.Task+VoidTask, ColossalManaged, Version=0.3.0.0, Culture=neutral, PublicKeyToken=null]].Execute()
   at ColossalFramework.Threading.Task.InternalExecute()
   at ColossalFramework.Threading.Dispatcher.RunTask(ColossalFramework.Threading.Task task)
   at ColossalFramework.Threading.Dispatcher.ProcessSingleTask(ColossalFramework.Threading.Task task)
   at ColossalFramework.Threading.Dispatcher.InternalProcessTasks()
   at ColossalFramework.Threading.Dispatcher.ProcessTasks()
   at ColossalFramework.Threading.ThreadHelper.FpsBoosterUpdate()
   at BehaviourUpdater.Updater.Update()

@originalfoo
Copy link
Member Author

@krzychu124 It's possible, ofc, but I didn't get any errors while testing during recent PRs.

1 is valid value so not sure why it's throwing that. I'll see if I can reproduce the error (at some point I'm going to create UI wrapper for all those dropdowns as their code is a massive mess at the moment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression old bug comes back or a new bug introduced in code that used to work. Settings Road config, mod options, config xml
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants