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

Add no games found message #254

Closed
wants to merge 5 commits into from

Conversation

lilyremigia
Copy link
Contributor

Also removes the greyed out scrollbar ^.^
Fix #235

@lilyremigia
Copy link
Contributor Author

@brliron

Copy link
Member

@brliron brliron left a comment

Choose a reason for hiding this comment

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

I had a few more things to comment on, but it turned out I was wrong on them (like, I thought the Timer was smart enough to stay alive even if you don't keep a reference to it, but it isn't).

x:Name="NoNewGamesAutoHideProgress"
Value="0"
Foreground="#FFB00606"

Copy link
Member

Choose a reason for hiding this comment

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

Unneeded empty line

@lilyremigia
Copy link
Contributor Author

@brliron CodeMaid

@brliron
Copy link
Member

brliron commented Mar 21, 2024

Isn't it better to keep the private variables and their getters stuck together, instead of splitting them with a newline? Like, for example, line 245 (old) / 250 (new) for stringdef_ / GetStringdef or line 200 (old) / 204 (new) for _isSelected / IsSelected.

@lilyremigia
Copy link
Contributor Author

I think CodeMaid is made with the pattern of private fields being together and then public properties being together, so you have a private section and then a public section.

@lilyremigia lilyremigia requested a review from brliron March 26, 2024 09:56
@brliron
Copy link
Member

brliron commented Mar 27, 2024

First, no, CodeMaid didn't do that. It didn't move things, it just added an empty line here and there.

But more importantly, CodeMaid made these change, but you're the one who committed them. You looked at every single one of them in a diff tool, and for each one, decided "yep, the old code was wrong and this version is better". Or at least I hope you did.

So why can't you say why you made these changes?

Btw, one of the changes I was wondering about is about adding "private" everywhere. The default seems to be internal. I thought that internal was more or less the same as private, and that it didn't matter. Looking on Google, I see "public within the assembly". What is an assembly? I guess it's either the whole exe or the compiled version of the current source file? And more importantly, why does Intellisense disagree with this definition? If I try to add something like this

Page4DropDownButtonBehavior abc = null;
abc.SaveG

from Page4, which is in the same source file as Page4DropDownButtonBehavior, and Intellisense doesn't autocomplete to SaveGamesJs, which makes me think I'm not allowed to call it from there (I have the older version of the code without the added private).

@lilyremigia
Copy link
Contributor Author

First, no, CodeMaid didn't do that. It didn't move things, it just added an empty line here and there.

Exactly, why I didn't think too much about it, and said CodeMaid is made with that pattern (in mind). Hence, why it probably looks awkward overall.

But more importantly, CodeMaid made these change, but you're the one who committed them. You looked at every single one of them in a diff tool, and for each one, decided "yep, the old code was wrong and this version is better". Or at least I hope you did.

If you painstakingly review every single line in a commit, good for you. I don't. I just test if what I have worked on works, then send it out to others to test it as well.

So why can't you say why you made these changes?

Code prettier go brr, probably looks better. Did it fuck up some code? No? 'kay.

Btw, one of the changes I was wondering about is about adding "private" everywhere. The default seems to be internal. I thought that internal was more or less the same as private, and that it didn't matter. Looking on Google, I see "public within the assembly". What is an assembly? I guess it's either the whole exe or the compiled version of the current source file? And more importantly, why does Intellisense disagree with this definition? If I try to add something like this
Page4DropDownButtonBehavior abc = null;
abc.SaveG
from Page4, which is in the same source file as Page4DropDownButtonBehavior, and Intellisense doesn't autocomplete to SaveGamesJs, which makes me think I'm not allowed to call it from there (I have the older version of the code without the added private).

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/accessibility-levels:
Top-level types, which are not nested in other types, can only have internal or public accessibility. The default accessibility for these types is internal.
Nested types, which are members of other types, are the following: enum members default is public, class members default to private, interface members default to public, and struct members default to private.

I think I will just be explicit with my accessibility, thank you.

@lilyremigia lilyremigia deleted the no-games-found branch June 17, 2024 07:14
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

Successfully merging this pull request may close these issues.

Add "No found games" message to configurator
2 participants