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

Rebases to /vg/station13 code. #23087

Merged
merged 30 commits into from
Mar 4, 2017
Merged

Conversation

PJB3005
Copy link
Contributor

@PJB3005 PJB3005 commented Jan 17, 2017

Bringing you hypothermia, MoMMIs, BEAMS, SM cascade, xenoarch and most importantly the R-UST.

🆑
rscadd: Rebased to /vg/station lighting code.
/:cl:

@@ -127,7 +127,9 @@

/mob/living/simple_animal/drone/cogscarab/New()
. = ..()
SetLuminosity(2,1)
#warn a
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what to do with cap is the issue.

Copy link
Member

@MrStonedOne MrStonedOne Jan 17, 2017

Choose a reason for hiding this comment

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

cap is strength.

or more accurately, how strong from normal (10) is the center. and then lum (the first arg) is normally just radius

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this is just supposed to be weak innate light, I was doing the best I could with the limited tool I had; ideally this would have been faint reddish light, like the scarab's eyes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured as such but there's no direct analog here.

@@ -18,7 +18,9 @@
START_PROCESSING(SSobj, src)
clockwork_caches++
update_slab_info()
SetLuminosity(2,1)
#warn falloff?
Copy link
Contributor

Choose a reason for hiding this comment

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

falloff was basically a cheap way to do light strength so just making this very weak light(which should be possible with this?), will be enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was midway through this and I didn't want to ask on IRC what the second argument was to ruin the surprise so.

@ChangelingRain
Copy link
Contributor

How does this compare to our current lighting, besides the visual difference? Mainly, can you do very weak light over a wide area, and very strong light over a small area?

@PJB3005
Copy link
Contributor Author

PJB3005 commented Jan 17, 2017

@Razharas all of those are warnings I put in as TODOs

@ChangelingRain: non shit backend for one thing.

There's no direct analog for cap in baylights, there's light power which is similar.

I don't actually understand the formulas of the lighting system, Mloc wrote that.

if(!top_atom.light_sources)
top_atom.light_sources = list()

top_atom.light_sources += src // Add ourselves to the light sources of our new top atom.
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace this and the if(!top_atom.light_sources) bit with LAZYADD(src, top_atom.light_sources) which will do the same thing

top_atom = source_atom
. = 1

if (istype(top_atom, /turf))
Copy link
Contributor

Choose a reason for hiding this comment

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

if(isturf(top_atom))

@ChangelingRain ChangelingRain added the Refactor Makes the code harder to read label Jan 17, 2017
@PJB3005
Copy link
Contributor Author

PJB3005 commented Jan 17, 2017

Woosh

@Incoming5643
Copy link
Contributor

Incoming5643 commented Jan 18, 2017

Anything that puts horrible gradients over our sprites is terrible and I dislike it unconditionally. Nearly all lighting system I've seen that aren't ours do this. I think we have the right idea now, keeping it simple and clean.

@WJohn
Copy link
Contributor

WJohn commented Jan 18, 2017

I don't know I really like the smooth lighting as opposed to blockyness. Although this may make us lose our wonderful gradual fade in/out brightness which is very pretty in and of itself.

If this includes colored lighting then I'll say yes for sure.

@GunHog
Copy link
Contributor

GunHog commented Jan 18, 2017

It is about time we updated to a prettier system! Darkness could be a lot more 'atmospheric', and colored lights can let us do prettier alarms and what not!

@PKPenguin321
Copy link
Contributor

This is gonna need a serious changelog.

Pretty excited for this.

@silicons
Copy link
Contributor

Dancefloors no longer making light
:(

@Incoming5643
Copy link
Contributor

From a performance standpoint I essentially cannot be a ghost and not drop a large number of frames with this on. I mean thankfully I can turn it off but still.

@francinum
Copy link
Contributor

Doesn't render for living mobs.

@MrStonedOne
Copy link
Member

MrStonedOne commented Jan 18, 2017

it didn't render because of our planemaster for lighting, removing that fixed it.

@PJB3005
Copy link
Contributor Author

PJB3005 commented Jan 18, 2017

All these people that are missing the joke.

Smh.

@Razharas Razharas added the Merge Conflict Adding upstream files to your repo via drag and drop won't resolve conflicts label Jan 18, 2017
@Razharas
Copy link
Contributor

I dont like how it looks, it makes snow loose all detail and just look like a uniform red colour

@Fox-McCloud
Copy link
Contributor

...snow?

@PJB3005
Copy link
Contributor Author

PJB3005 commented Jan 18, 2017

That fire in the bottom right does kind of look like snow now that you mention it.

@Razharas
Copy link
Contributor

It doesnt matter if its snow or some other white stuff covering the floor, originally it had some detail drawn on it but with this light it became straight red with nothing in it

@PKPenguin321
Copy link
Contributor

PKPenguin321 commented Jan 18, 2017

Well I mean it kind of makes sense, go stare at the sun and see if you can still make out the colors and details of the sky around it

@PJB3005
Copy link
Contributor Author

PJB3005 commented Jan 18, 2017

Also I think that may be the fire overlay itself blocking the snow somewhat too.

@GunHog
Copy link
Contributor

GunHog commented Jan 18, 2017

It is probably foam, not snow!

@Fox-McCloud
Copy link
Contributor

There is noooo snowww in the pictureee.

@PJB3005
Copy link
Contributor Author

PJB3005 commented Jan 18, 2017

Whoosh.

@ghost
Copy link

ghost commented Mar 4, 2017

I assume that I'm not getting a cohesive answers then

@XDTM
Copy link
Contributor

XDTM commented Mar 4, 2017

Yes, it is lighting only, which you could tell if you glanced at the code

@ghost
Copy link

ghost commented Mar 4, 2017

I saw the lighting stuff but I didn't know if that was all. I don't know VG code. Thank you for the answer.

@Incoming5643
Copy link
Contributor

Please make the changelog entry actually correct. It's not the place for sarcasm.

@GunHog
Copy link
Contributor

GunHog commented Mar 4, 2017

Yes, it is lighting only, which you could tell if you glanced at the code

Because we totally expect every player on Github, including non-coders, to read through ~1300 new lines of code across 159 files to determine that the only change is lighting.

This is akin to a renowned physicist hosting a high-school science class while having no experience as a teacher. His expectations are going to be severely off the mark.

@Poojawa
Copy link
Contributor

Poojawa commented Mar 4, 2017

I mean, you have to scroll completely through all comments to make a comment asking what's going on.

I'm sure he'll change it before it's merged.

@ghost
Copy link

ghost commented Mar 4, 2017

Yeah, but you don't have to READ them

@Bawhoppen
Copy link
Contributor

that is a nice hearty sized gif

also please don't merge this

@PJB3005
Copy link
Contributor Author

PJB3005 commented Mar 4, 2017

RIOT

@ChangelingRain
Copy link
Contributor

You're gonna have to make it serious at some point!
And also seriously is the memory leak fixed or what

@PJB3005
Copy link
Contributor Author

PJB3005 commented Mar 4, 2017

There's no memory leak from what I can tell.

@ChangelingRain
Copy link
Contributor

In that case, it's time forrrrrrr

@ChangelingRain ChangelingRain merged commit 3c08c4f into tgstation:master Mar 4, 2017
@ChangelingRain
Copy link
Contributor

I sure hope you're right!

@PJB3005
Copy link
Contributor Author

PJB3005 commented Mar 4, 2017

oh boy.

@GunHog
Copy link
Contributor

GunHog commented Mar 4, 2017

YEEEEEEEEEEEEEEEEEEES

@ghost
Copy link

ghost commented Mar 4, 2017

PogChamp

@chanoc1
Copy link
Contributor

chanoc1 commented Mar 4, 2017

we did it boys

@imtakingabreakdontatme
Copy link
Contributor

Shouldn't we have tested this more if we thought there was a memory leak

@Incoming5643
Copy link
Contributor

@KorPhaeron
Yep

@PKPenguin321
Copy link
Contributor

Wooooooooooooooooo

@Okand37
Copy link
Contributor

Okand37 commented Mar 7, 2017

Very pretty.

@PKPenguin321
Copy link
Contributor

Zombie!

@GunHog
Copy link
Contributor

GunHog commented Mar 8, 2017

@PJB3005 How do I disable permanent lighting in a shuttle area?

@PJB3005
Copy link
Contributor Author

PJB3005 commented Mar 8, 2017

@GunHog give them dynamic_lighting = TRUE

@GunHog
Copy link
Contributor

GunHog commented Mar 8, 2017

I did so locally for /area/shuttle/auxillary_base, but it seems that something at runtime is setting it back to FALSE.

@MrStonedOne
Copy link
Member

just do a find for shit that sets that, i think something in shuttle code fucks with that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Reproducing Nobody will fix your issue unless you do this Refactor Makes the code harder to read
Projects
None yet
Development

Successfully merging this pull request may close these issues.