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

Added Aero Feature. #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added Aero Feature. #10

wants to merge 1 commit into from

Conversation

bwcsemaj
Copy link
Contributor

@bwcsemaj bwcsemaj commented Oct 7, 2018

Added aero feature to Borderless Scene. Got rid of the previous solution. Added some variables. One of which could be a property.

BUFFER_SPACE is used to make it so you don't have be exactly in the corner in order to snap in the corner. Right now I set it to 100 but it can be anything greater than or equal 0.

I added a Direction Enum to help with the calculation. The direction values also could be boolean value though I found it more nicer with the Enum.

@goxr3plus
Copy link
Owner

@bwcsemaj Thank you so much .... can you keep the same format and only add the code you modified because it's hard for me to see the differences in order to merge :'(

@goxr3plus
Copy link
Owner

@bwcsemaj Well maybe found a way to see better the changes https://github.com/goxr3plus/FX-BorderlessScene/pull/10/files?utf8=%E2%9C%93&diff=split&w=1

but remember in the future to keep the format as it was because project owners are starting to cry when format is changed :)

@goxr3plus
Copy link
Owner

Okay so today i took a good look on it... have you tested that everything works fine before i merge :)?

@bwcsemaj
Copy link
Contributor Author

bwcsemaj commented Nov 23, 2018

I know my code doesn't break anything. However, it isn't pretty. I would take my code that I added, copy it, and implement it so it doesn't look like crap. You might want to actually give access to what I called BUFFER_SPACE which that name doesn't really make too much sense but I didn't know what else to call it. 100 for some people might be too much or too little in my case. It should be a property though that people can change.

Its been a while but I have taken your project idea and implemented it into my game so one less dependency to work with. I do plan on reworking all of it sometime after I iron out my game a bit. I say this because there was a bug regarding the Delta object again, I forget where in the code, but I added a null check for the time being.

You might want to organize my implementation more. I know mine isn't too pretty to look at and could be better worked. I added a Direciton object that you could also take advantage of or replace with just Strings. I wish I took sometime adding more comments on what was happening in the code. It isn't too hard to understand but would save sometime for others to look at.

@goxr3plus
Copy link
Owner

goxr3plus commented Nov 25, 2018 via email

@goxr3plus
Copy link
Owner

We are having some conflicts with the latest release of FX-Borderless Scene can you help me merge your code :) ?

@bwcsemaj
Copy link
Contributor Author

bwcsemaj commented May 11, 2019

I wouldn't merge it. Definitely take the logic behind the calculation of size/position but I wouldn't merge what I suggested. I'm sure just glancing at my code now there is a slightly more efficient way of going about it too so it doesn't take as much lines.

Now lets assume I thought merge was good one, I would help merge but there are other issues that need to be addressed like Delta's fields using Double rather than double (which not properly initialized causes needless null checks), restoring, minimizing, keeping track what state it is in (whether snapped/minimized/maximized/"normal")...

I think the overall class would have to be reworked. The underlying idea that you have for this library is really good but just somethings that I think need to be rethought/re implemented.

I'm quite busy trying to wrap my game up. All my focus has to go into that for now, but if I get some free time and feel like coding I'll try post some code or maybe even a pull request of what I think the project should be more of but I don't even know when that will be.

@goxr3plus
Copy link
Owner

Thank you, i will do it then :)

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.

2 participants