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

Differentiate Labs and Stable & Avoid Multiple Subscriptions #333

Merged
merged 35 commits into from
Jun 30, 2019

Conversation

krzychu124
Copy link
Member

@krzychu124 krzychu124 commented May 24, 2019

Added suffix LABS to differentiate labs and stable versions.

  • when sources built using Release LABS configuration Version string is extended with LABS text e.g. TM:PE 10.21 LABS
  • visible in Content Manager, Mod options tab and label of the mod main menu in-game (label is built from TM:PE + that Version string)

Fixes #326, Fixes #386

@krzychu124 krzychu124 added Usability Make mod easier to use LABS TM:PE LABS branch technical Tasks that need to be performed in order to improve quality and maintainability UI User interface updates labels May 24, 2019
@krzychu124 krzychu124 added this to the 10.21 milestone May 24, 2019
@krzychu124 krzychu124 self-assigned this May 24, 2019
@originalfoo
Copy link
Member

@krzychu124 Want me to add the changes suggested above to the PR?

Separated out the branch string, so we can now distinguish between DEBUG, LABS and STABLE. Also minor code rearranging to aid readability.
Using the new LABS token allows us to dynamically add other version of TM:PE (either STABLE or LABS as applicalbe) to the incompatible mod checker. This still won't detect local install versions, but it will drastically reduce number of users who have more than one TM:PE subbed and subsequently reduce the issues with non-saving road config.
@originalfoo
Copy link
Member

originalfoo commented May 28, 2019

Now that we have LABS token and thus a way to distinguish between two builds, I've also set it up to dynamically add the other version (LABS or STABLE, or both, depending on build) to incompatible mods checker. It still won't detect local builds but will at least drastically reduce number of end users having two versions of the mod installed and subsequent issues with road customisations not saving.

@originalfoo
Copy link
Member

originalfoo commented May 28, 2019

An object reference is required for the non-static field, method, or property 'TrafficManagerMod.Version'

Uhm, what did I do wrong?

Ah, it's actually in VersionLabel.cs:

namespace TrafficManager.UI.MainMenu {
	public class VersionLabel : UILabel {
		public override void Start() {
			size = new Vector2(MainMenuPanel.SIZE_PROFILES[0].MENU_WIDTH, MainMenuPanel.SIZE_PROFILES[0].TOP_BORDER); // TODO use current size profile
			text = "TM:PE " + TrafficManagerMod.Version; // <-- this line ########
			relativePosition = new Vector3(5f, 5f);
			textAlignment = UIHorizontalAlignment.Left;
		}
	}
}

@FireController1847 FireController1847 changed the title 326 differentiate labs and stable - Fixes #326 Differentiate Labs and Stable May 28, 2019
VersionLabel.cs references the `Version` value, so made it static like it was before (not ideal, but not a regression either). Also added L, D, or S to denote build branch which will be useful for support purposes.
@originalfoo
Copy link
Member

originalfoo commented May 28, 2019

Ok, that should fix the static error.

Here's summary of changes I made:

  1. Split out the branch in to its own field (property? not sure about naming convention)
  2. Updated the TM:PE toolbar caption so it includes -L, -D, or -S suffix denoting which build is active
  3. Depending on build, add other TM:PE version(s) to the incompatible mod checker to help reduce number of users with multiple TM:PE versions installed. This still won't detect local builds, but it will reduce support workload of the "My road customisations aren't saving" variety.

@krzychu124 Can you check and see if it looks ok to you?

@originalfoo
Copy link
Member

Ugh, just realised I did the incompatible mod checker bit wrong... Need to do some tweaks.

Fixed blunder with location of the additions to incompatible mods list.
This adds the branch to the title on the incompatible mods dialog. I've updated language files as applicable; most were untranslated, except `fr` and `ko` - `ko` didn't include the word `Traffic Manager` and `fr` had the translation at the start so easy to modify.
@originalfoo
Copy link
Member

originalfoo commented May 28, 2019

Ok, two more updates:

  1. Fixed the blunder with incompatible mod checker
  2. Changed title bar of incompatible mods dialog to include branch (otherwise it gets confusing af as you won't know which branch of TM:PE is throwing the warnings)

Tagging: #211, #306, #190, #149

@originalfoo originalfoo changed the title Differentiate Labs and Stable Differentiate Labs and Stable, and avoid multiple subscriptions May 28, 2019
Changed workshop ids to ulong.
@originalfoo
Copy link
Member

Does help if I define workshop ids as ulong lol.

branch checker

originalfoo
originalfoo previously approved these changes May 28, 2019
Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

Ok, LGTM, but then it would... Needs additional reviewers.

TLM/TLM/TrafficManagerMod.cs Outdated Show resolved Hide resolved
@originalfoo
Copy link
Member

originalfoo commented May 28, 2019

Things to check in review:

  • Content Manager > Mods shows branch (DEBUG, LABS, or STABLE) for your local build
  • Mod options screen: mods list in sidebar shows branch for your local build
  • TMPE toolbar in game shows abbreviated branch (might be too ugly, lmk what you think)
  • Mod incompatibility checker detects duplicate subscriptions (it won't detect local build)
  • Mod incompatibility dialog shows branch in title bar
  • Mod incompatibility dialog shows branch on any duplicate subscriptions listed as conflicts

@originalfoo
Copy link
Member

originalfoo commented Jun 20, 2019

From #386:

  • Rip out any remaining use of Linq, it doesn't work very well on OS X platform
  • Add in some try .. catch wrappers to prevent exceptions reaching end users

@originalfoo originalfoo added the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Jun 20, 2019
@dymanoid
Copy link
Contributor

  • Rip out any remaining use of Linq, it doesn't work very well on OS X platform

AFAIK there are no issues with Linq in Mono, regardless of the target platform.

Revert to Path.GetFileName() - will fix formatting later.
Use plugin.userModInsstance to get mod name.
Wrap release-only `using` clause in #if/endif
Tidied up `using` clauses, fixed typo in a method summary, autoformat doc.
Catch and log exceptions in main PerformModCheck() method, autoformat doc, removed Linq as no longer required.
@originalfoo originalfoo removed the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Jun 26, 2019
@originalfoo
Copy link
Member

Tested everything I can think of and it seems to be working fine now.

Would appreciate another set of eyes to check over stuff before merging.

I've uploaded a fresh test version of the mod to workshop (published as RELEASE LABS) - it will check for both offline and other workshop versions: https://steamcommunity.com/sharedfiles/filedetails/?id=1781170897 and if you also do a local build set to DEBUG that will only check for the workshop versions.

@originalfoo
Copy link
Member

Fixes #386

@originalfoo originalfoo added the Localisation Localised text and features label Jun 26, 2019
Copy link
Collaborator

@FireController1847 FireController1847 left a comment

Choose a reason for hiding this comment

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

Other than these two I think it looks good

TLM/TLM/Resources/lang.txt Show resolved Hide resolved
TLM/TLM/TrafficManagerMod.cs Show resolved Hide resolved
@originalfoo originalfoo added the waiting An approved PR that's waiting for 3 days before available for merge. label Jun 28, 2019
@FireController1847 FireController1847 added waiting An approved PR that's waiting for 3 days before available for merge. and removed waiting An approved PR that's waiting for 3 days before available for merge. labels Jun 30, 2019
@krzychu124
Copy link
Member Author

Tested, LGTM 👍

@FireController1847 FireController1847 merged commit dbc21ca into master Jun 30, 2019
@originalfoo originalfoo removed the waiting An approved PR that's waiting for 3 days before available for merge. label Jul 1, 2019
@krzychu124 krzychu124 deleted the 326-differentiate-labs-and-stable branch July 27, 2019 22:14
@originalfoo originalfoo added the COMPATIBILITY Mod (in)compatibility / checker label Aug 13, 2019
@originalfoo originalfoo added the Toolbar The main TMPE toolbar label Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMPATIBILITY Mod (in)compatibility / checker LABS TM:PE LABS branch Localisation Localised text and features technical Tasks that need to be performed in order to improve quality and maintainability Toolbar The main TMPE toolbar UI User interface updates Usability Make mod easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mod compatibility checker - null issue Differentiate Stable vs. Labs in content manager
4 participants