[Added] Action Special to reveal lines on automap

Moderator: Developers

Re: Action Special to reveal lines on automap

Postby Rachael » Thu May 17, 2018 5:54 pm

It looks like what happened was she had these files open before a merge, but saved them *after* a merge.

In the future, if you have this happen, you save the files to the version you are modifying directly, do not do a merge before saving it, and then do a merge, and fix conflicts if necessary.
User avatar
Rachael
QZDoom + Webmaster
 
Joined: 13 Jan 2004

Re: Action Special to reveal lines on automap

Postby RockstarRaccoon » Thu May 17, 2018 8:52 pm

Graf Zahl wrote:How did you commit this? It has some line ending conflicts which make it impossible to merge without whacking the repo. I fortunately noticed before pushing and creating a real problem.
Nash wrote:Looks like a very dirty PR, for a feature this minimal. I'd recommend pulling the latest commit from master and redoing everything cleanly.
Rachael wrote:It looks like what happened was she had these files open before a merge, but saved them *after* a merge.

In the future, if you have this happen, you save the files to the version you are modifying directly, do not do a merge before saving it, and then do a merge, and fix conflicts if necessary.

Yeah, sorry about that. I'm going to stop asking the people on Discord if I'm doing things right, because apparently "whoever merges this in will fix it" was clearly not the correct response.

Like I said, I haven't used GitHub, or similar systems enough to think about stuff like synching my fork to your commits and synching VisualStudio to my fork. I'll fix it if you need me to, and go ahead and set that up for the future...
User avatar
RockstarRaccoon
Totally Babies
 
Joined: 31 Jul 2016

Re: Action Special to reveal lines on automap

Postby Rachael » Thu May 17, 2018 9:04 pm

What got you was _mental_'s #include removals but I thought that it was pretty old that it would not have affected you this badly.

When you asked that question, I thought you were referring to your code, directly, not a botched Git merge.
User avatar
Rachael
QZDoom + Webmaster
 
Joined: 13 Jan 2004

Re: Action Special to reveal lines on automap

Postby Graf Zahl » Fri May 18, 2018 12:05 am

The changelog looks dirtier than just cherry picking the two commits. But even then I got some problems with line endings. This is annoying because the Windows client totally throws up if it is set to line ending adjustment to native format and then getting data that is mixed Windows/Unix text. It's nearly impossible to do a clean recommit after that. I already had to fix such a PR a few months ago by a forced push to master which is always a problematic thing.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Action Special to reveal lines on automap

Postby RockstarRaccoon » Fri May 18, 2018 12:35 am

Well, now I'm confused: I could've sworn I was using your latest github version, but you're right. Somehow I must've gotten a version from quite some time ago...

Ugh, what a mess... Anyway, gonna try and fix this...

Edit: ok, I outright deleted my fork and I'm gonna work in my changes from here. No idea how this happened, but it looks like I somehow got a VERY old fork of it, perhaps the 3.2 maintenance build or something... Anyway, working it out right now...
User avatar
RockstarRaccoon
Totally Babies
 
Joined: 31 Jul 2016

Re: Action Special to reveal lines on automap

Postby RockstarRaccoon » Fri May 18, 2018 1:23 am

Here we go!
https://github.com/coelckers/gzdoom/pull/484

This one shouldn't have any problems. Sorry for the trouble and thanks for being patient with me. :3
User avatar
RockstarRaccoon
Totally Babies
 
Joined: 31 Jul 2016

Re: Action Special to reveal lines on automap

Postby Rachael » Fri May 18, 2018 1:50 am

This looks much better.

However, for these lines: https://github.com/RockstarRaccoon/GZRa ... 2664-L2691

These lines make it painfully obvious that the feature is as yet incomplete. I do not know how Graf feels about including that massive comment block, but were I in his shoes I would be asking what is planned for those lines, and what you may have trouble with for completing the feature.

Other than that, nothing jumped out at me, but I am half asleep right now so I can't give the code a truly fair review.
User avatar
Rachael
QZDoom + Webmaster
 
Joined: 13 Jan 2004

Re: Action Special to reveal lines on automap

Postby RockstarRaccoon » Fri May 18, 2018 1:59 am

Oh crap! I meant to delete those! Please delete those lines if you don't like them being there.

Yeah, one of the things that bugged me in Metroid Doom was that in Metroid Prime, hatch doors on the map are colored based on the beam weapon you can use, so when you get that beam you can see where you couldn't go before, but GZDoom doesn't have any such feature, so I was left with the player having to guess. I was planning on implementing a feature where the Mapper could define some custom colors in MAPINFO and use the "Force Automap Color" to make them show up. They would've used values in the 20s, because I was pretty sure we'd never need those, but I was at a point where I really need to get back to working on the actual mapping of Membrane and I already had the feature I needed halfway implemented (revealing lines), so I figured I'd look back at it when I put in the new special or whatever.
User avatar
RockstarRaccoon
Totally Babies
 
Joined: 31 Jul 2016

Re: Action Special to reveal lines on automap

Postby Rachael » Fri May 18, 2018 2:03 am

I will be busy for a good portion of tomorrow, but if I have some time I'll take a look at it and review it. I might even be able to teach you a few Git tricks to help you with future development. ;)
User avatar
Rachael
QZDoom + Webmaster
 
Joined: 13 Jan 2004

Re: Action Special to reveal lines on automap

Postby RockstarRaccoon » Fri May 18, 2018 9:43 am

Okay Rachael, I'll be around. :3
User avatar
RockstarRaccoon
Totally Babies
 
Joined: 31 Jul 2016

Re: Action Special to reveal lines on automap

Postby RockstarRaccoon » Fri May 18, 2018 10:17 pm

Comments Removed. It should be perfectly good now.

Attached also is a map which shows a basic demonstration of the "Revealed" flag and the "forceAMap" field.
Attachments
MAP01.wad
(4.97 KiB) Downloaded 14 times
User avatar
RockstarRaccoon
Totally Babies
 
Joined: 31 Jul 2016

Re: Action Special to reveal lines on automap

Postby _mental_ » Sat May 19, 2018 2:53 am

This is how in my opinion the given feature should be implemented.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: Action Special to reveal lines on automap

Postby Graf Zahl » Sat May 19, 2018 3:17 am

Yes, that looks a lot better.

Regarding the line endings, what tools are you using? If it screws up the line endings it must bypass some parts of how Git processes files before committing them.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Action Special to reveal lines on automap

Postby _mental_ » Sat May 19, 2018 3:34 am

I believe that file upload via GitHub webpage may screw up line ending depending on setup. Local system's CR/LF defaults and repository's .gitattributes entries affect this.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: Action Special to reveal lines on automap

Postby Nash » Sat May 19, 2018 3:35 am

_mental_ wrote:This is how in my opinion the given feature should be implemented.


Looks good!
User avatar
Nash
Nash Muhandes
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia

PreviousNext

Return to Closed Feature Suggestions

Who is online

Users browsing this forum: No registered users and 1 guest