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

Displaying Ties and OTL in Records #446

Open
mamamia5x opened this issue May 8, 2023 · 4 comments
Open

Displaying Ties and OTL in Records #446

mamamia5x opened this issue May 8, 2023 · 4 comments

Comments

@mamamia5x
Copy link
Contributor

So I was playing hockey GM with a league that allowed OTL's and ties. When displaying the record, there is no distinction from OTL and ties if you have 0 OTL and 1 or more ties. Like in my case, I have one tie and no OTL's. The record is really confusing then, making me believe that I have 1 OTL, not 1 tie.

image

let record = `${won}-${lost}`;
	if (typeof otl === "number" && !Number.isNaN(otl) && otl > 0) {
		record += `-${otl}`;
	}
	if (typeof tied === "number" && !Number.isNaN(tied) && tied > 0) {
		if (typeof otl === "number" && !Number.isNaN(otl) && otl == 0){
			record += `-${otl}`;
		}
		record += `-${tied}`;
	}

I figured out how to change this in the helper.js file at line 1337 with the code above. It now displays like this:
image

This was my type of fix, which works on my roster page. This doesn't work on the dashboard page and draft lottery page, and I can't seem to figure out what else generates the record text.

What other files need to be changed in other for the records to be displayed properly?

@dumbmatter
Copy link
Member

Yeah it's confusing to format records with OTL and ties enabled. Not sure if any IRL leagues have both, and if so how they show them?

You did find the correct place to change how records are formatted, the formatRecord function in helpers.ts. And you are also correct that it doesn't seem to work everywhere. That's because originally formatRecord didn't exist and the records were just formatted inline everywhere, and when I wrote formatRecord I tried to use it everywhere, but clearly I missed some places. The solution would be to change those places to use formatRecord.

So far you identified these places:

And I also found this similar one from a quick search of the codebase:

If you want to send a PR to fix this, feel free! If not, let me know and I'll fix it myself - it's an easy fix for me, but if you want to have fun and try to do it yourself, you can :)

@mamamia5x
Copy link
Contributor Author

I'll try myself, I could use the experience with TS

@mamamia5x
Copy link
Contributor Author

mamamia5x commented May 10, 2023

Going through the files, this fix also shows when OTL's are disabled. In a league where there are ties and no OTL's, it'll display 5-2-0-1 instead of 5-2-1 for example. Is there a quick way for me to check if OTL is enabled?

Edit: or is there a way to store OTL's as null (when its disabled), then check if the value is null?

@dumbmatter
Copy link
Member

No, there's two complications:

  • the UI process doesn't have access to the otl setting currently
  • the history of the otl setting isn't currently stored, so just looking at the current setting

Both of those could be solved relatively easily, but there's another problem:

  • settings with history do not have history exposed to the ui currently

That would be a bit more work to fix.

And I'm not inclined to do any of this, because the current formatRecord behavior works good enough and doesn't require it. Sorry if that makes things worse for your version :)

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

No branches or pull requests

2 participants