Putting the 'role' back in role-playing games since 2002.
Donate to Codex
Good Old Games
  • Welcome to rpgcodex.net, a site dedicated to discussing computer based role-playing games in a free and open fashion. We're less strict than other forums, but please refer to the rules.

    "This message is awaiting moderator approval": All new users must pass through our moderation queue before they will be able to post normally. Until your account has "passed" your posts will only be visible to yourself (and moderators) until they are approved. Give us a week to get around to approving / deleting / ignoring your mundane opinion on crap before hassling us about it. Once you have passed the moderation period (think of it as a test), you will be able to post normally, just like all the other retards.

Pillars of Eternity Beta Discussion [GAME RELEASED, GO TO NEW THREAD]

Arkeus

Arcane
Joined
Oct 9, 2012
Messages
1,406
In before this code thing ends up being like the Pallegina voice acting
Btw, do we have confirmation that Caedung's been scrapped?

If so, that's really sad :(
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California
what im trying to figure out is whether the obsidian programmers know their code is a travesty and just don't care, or if they don't realise how bad it is

because personally, a 4,000-line class like CharacterStats.cs would get me fired

You could ask Adam Brennecke or we can summon Anthony Davis

Which I just did.

What's do you have a problem with, the line count, or the content?

I haven't seen the code - so I can't comment on it. How did you guys get it?
 

HiddenX

The Elder Spy
Patron
Joined
May 20, 2006
Messages
1,655
Location
Germany
Divinity: Original Sin Shadorwun: Hong Kong
Grognards program in Ansi C or C++, functional style (not OO style) and get the .exe-file on a 1.44 MB disk :)
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California
what im trying to figure out is whether the obsidian programmers know their code is a travesty and just don't care, or if they don't realise how bad it is

because personally, a 4,000-line class like CharacterStats.cs would get me fired

You could ask Adam Brennecke or we can summon Anthony Davis

Which I just did.

What's do you have a problem with, the line count, or the content?

I haven't seen the code - so I can't comment on it. How did you guys get it?

I backtracked a bit in this thread and saw mention of a decompile thread at obsidian, but I can't find it over there. Someone is going to have to give me some context.
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California

Thanks, I skimmed those, but I'm not seeing the file and the particular issue that people are talking about recently in this thread. The question I was summoned for. It might be in there, but what I saw was mod stuff, inventory mod, ai structure, and some talk about c#.

Anything else?
 

Shannow

Waster of Time
Joined
Sep 15, 2006
Messages
6,386
Location
Finnegan's Wake

mutonizer

Arcane
Patron
Joined
Sep 4, 2014
Messages
1,041
Did you decompile and checked the source a bit? Or do you mean a specific class within the source?
 

coffeetable

Savant
Joined
Dec 18, 2012
Messages
446

Thanks, I skimmed those, but I'm not seeing the file and the particular issue that people are talking about recently in this thread. The question I was summoned for. It might be in there, but what I saw was mod stuff, inventory mod, ai structure, and some talk about c#.

Anything else?
http://forums.obsidian.net/topic/68573-decompiling-the-beta/
If any of the Obsidian programmers happen across this: how did CharacterStats.cs and StatusEffects.cs come about? I know some of it is bloat from the compiler, but both are north of 4,000 lines. Do all games look like this internally? How do you debug them?
i did originally link to a pastebin of CharacterStats.cs, then took it down. suffice it to say it's not pretty and the vast majority of it can't be blamed on compiler bloat
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California

Thanks, I skimmed those, but I'm not seeing the file and the particular issue that people are talking about recently in this thread. The question I was summoned for. It might be in there, but what I saw was mod stuff, inventory mod, ai structure, and some talk about c#.

Anything else?
http://forums.obsidian.net/topic/68573-decompiling-the-beta/
If any of the Obsidian programmers happen across this: how did CharacterStats.cs and StatusEffects.cs come about? I know some of it is bloat from the compiler, but both are north of 4,000 lines. Do all games look like this internally? How do you debug them?
i did originally link to a pastebin of CharacterStats.cs, then took it down. suffice it to say it's not pretty and the vast majority of it can't be blamed on compiler bloat

If I can't see it, I can't talk about it. I can talk in generalizations, but that won't convince anyone. I certainly wouldn't judge anything based on decompiled code.

Without seeing anything in particular, the thing I would keep in mind is that Designs change - the system doesn't always end up the same as you imagine or build.
 

mutonizer

Arcane
Patron
Joined
Sep 4, 2014
Messages
1,041
Yea, I'm by far not a professional coder and my formation was from another era, but I do still poke my fingers in code from time to time, especially lately. Most of my comments were from the fact that, for example, I can take Wasteland 2 decompiled sources (unity based just like PoE) and find it perfectly clear, clean and follow it without problems in any direction, and instantly understand it, while in the PoE version of that, I'm going "wtf" almost everywhere.

As I said before, it's either because I'm too dumb for the smartness of it, or too smart for the dumbness of it.

As long as it runs properly though, who cares right? The fact that there are some major crazy shit bugs (and I mean crazy, as in stupid simple shit that should never happen if anyone spent a coffee break to clean this stuff up) in the version they released supposedly 4 months before release doesn't help...
 

coffeetable

Savant
Joined
Dec 18, 2012
Messages
446
If I can't see it, I can't talk about it. I can talk in generalizations, but that won't convince anyone. I certainly wouldn't judge anything based on decompiled code.

Without seeing anything in particular, the thing I would keep in mind is that Designs change - the system doesn't always end up the same as you imagine or build.

aight, for your eyes only:

http://pastebin.com/PgtgPDPC

decompiled C# is eminently readable. and while some of this is the compiler inlining things or unrolling loops, last i checked compilers don't habitually write classes like this:

RO7r8QP.png


this is a single class
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California
If I can't see it, I can't talk about it. I can talk in generalizations, but that won't convince anyone. I certainly wouldn't judge anything based on decompiled code.

Without seeing anything in particular, the thing I would keep in mind is that Designs change - the system doesn't always end up the same as you imagine or build.

aight, for your eyes only:

http://pastebin.com/PgtgPDPC

decompiled C# is eminently readable. and while some of this is the compiler inlining things or unrolling loops, last i checked compilers don't habitually write classes like this:



this is a single class

And?

Skimming this, I don't see anything that doesn't belong. Some classes are light weight, some are not. You could make an argument (from the outside) that some structs could be moved out - but it's just a definition.

Seriously, other than the line count, what do you think is wrong with this? If it was written with old school bracing and spacing, the line count would be around 1000. Line count doesn't mean shit.


This class might not even be that heavyweight - you would have to add everything up to see (which I have not done). If for some reason you wanted to - and if there was a reason to, I suppose you could break every last thing out into it's own class file - but there is a cost for that and you aren't really changing anything if Character Stats still needs everything here, because you will still need to include it.

Have you actually gone through and looked for anything technically wrong?

We could go through this file bit by bit, but I would rather play WL2 than argue on the internet. If you don't like the code, that's cool, go write something better - the industry always needs good ideas.
 

Immortal

Arcane
In My Safe Space
Joined
Sep 13, 2014
Messages
5,062
Location
Safe Space - Don't Bulli
Pretty Sure Anthony would mostly agree with this.. Curious about his opinion.. He is actually a game dev after all maybe the standards are different?

My Bullshit opinions:
Some code seems to be following a very strict concise pattern and some of it seems to be a little heavy on the in-lining.. Not sure how I feel about those Enums being shoved in the class namespace like that but honestly that class isn't that bad..

If you guys are actually guffawing at a 6000 line class.. I would argue you haven't worked on a large project and you have never had a strict deadline that was in jeopardy..
Upset about the patterns not being consistent? Your company must also have never hired contractors or new employees and never had a manager who was getting yelled at by another manager.. Oh and Unity doesn't exactly make things easier.

In a perfect world we want proper patterns and a consistent stack that is always modular and decoupled as much as possible.. The reality is.. Crunch times happen.. Requirements change and shit happens..

Anyone having trouble navigating that class is not a very strong developer.. Commented out code or code not being used in a BETA release that isn't even being deployed?! HOW DARE THEY! (I checked in code on Friday that many -redacted- customers are using right now.. There's tons of harmless code (mostly commented but not all) in that check in for future stories in our backlog that didn't make it in this sprint.. so what?)

Also if anyone is bitching about lack of code form or comments.. you do understand what decompilation does right? The fact that IL maintains class and method signatures should be praised.. you guys should check out some decompiled byteCode sometime.. :roll:
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California
Pretty Sure Anthony would mostly agree with this.. Curious about his opinion.. He is actually a game dev after all maybe the standards are different?

My Bullshit opinions:
Some code seems to be following a very strict concise pattern and some of it seems to be a little heavy on the in-lining.. Not sure how I feel about those Enums being shoved in the class namespace like that but honestly that class isn't that bad..

If you guys are actually guffawing at a 6000 line class.. I would argue you haven't worked on a large project and you have never had a strict deadline that was in jeopardy..
Upset about the patterns not being consistent? Your company must also have never hired contractors or new employees and never had a manager who was getting yelled at by another manager.. Oh and Unity doesn't exactly make things easier.

In a perfect world we want proper patterns and a consistent stack that is always modular and decoupled as much as possible.. The reality is.. Crunch times happen.. Requirements change and shit happens..

Anyone having trouble navigating that class is not a very strong developer.. Commented out code or code not being used in a BETA release that isn't even being deployed?! HOW DARE THEY! (I checked in code on Friday that many -redacted- customers are using right now.. There's tons of harmless code (mostly commented but not all) in that check in for future stories in our backlog that didn't make it in this sprint.. so what?)

Also if anyone is bitching about lack of code form or comments.. you do understand what decompilation does right? The fact that IL maintains class and method signatures should be praised.. you guys should check out some decompiled byteCode sometime.. :roll:

This is definitely true.

You might have a senior do the TDD and the initial implementation - someone like Tim Cain, Steve Weatherly, or Dan Spitzely. Their implementation is great. They move off to the next system and then changes or modifications to the system sometimes fall to a junior. The code will get reviewed of course, but a lot of the time you look at the change and why it was made, and then you look at your deadline and say, "That'll do, pig."

You know sometimes that when you are doing this that you are accumulating "code debt". In a perfect world, you will pay that debt off and get the system as clean and lean as you want. That doesn't always happen - and in this particular case, I'm still not seeing anything that CharacterStats wouldn't need.

Obviously as you gain experience, you also code systems in such a way where they can be easily changed and adapted (hopefully). The price of that adaptability is leanness.


Story time.
I have OCD. No surprise, many engineers do.

A long time ago, when I first started out - working for Enron, Dow Chemical, Xerox, Netscape, etc. I was always deleting code and making sure whatever I checked in was as clean as clean could be. My OCD demanded it of me.

As time went on, I realized I was constantly going back and changing things BECAUSE.... designs and specifications change, even in the non-gaming industry (the gaming industry is about 100x worse due to the nature of the product). It took awhile, but I eventually trained myself to comment out code, if def out code, or just remove the call and let the compiler no-op it.

I still hate looking at it - it keeps me riled up. Then I remind myself, that if they decide to change Sprint, or Fireball,or whatever example, I'm ready.
 

mutonizer

Arcane
Patron
Joined
Sep 4, 2014
Messages
1,041
Always glad to know that the answer is that I'm too dumb for it's smartness :)

Other than that, I don't know. It seems to handle both PC and NPC and monsters and anyone with stats equallty. Handles noise level and lore status, but not some core abilities, handles some damage but not other. Handles statics related to combat, some related to weapons, some to characters. Has methods just to calculate randoms, without any link to anything within the class or any variables in there either. Not saying it doesn't work, but in this day and age, aren't you supposed to, you know, base, derive, organize and whatnot?
 

Anthony Davis

Blizzard Entertainment
Developer
Joined
Sep 7, 2007
Messages
2,100
Location
California
Always glad to know that the answer is that I'm too dumb for it's smartness :)

Other than that, I don't know. It seems to handle both PC and NPC and monsters and anyone with stats equallty. Handles noise level and lore status, but not some core abilities, handles some damage but not other. Handles statics related to combat, some related to weapons, some to characters. Has methods just to calculate randoms, without any link to anything within the class or any variables in there either. Not saying it doesn't work, but in this day and age, aren't you supposed to, you know, base, derive, organize and whatnot?

Absolutely - when it makes sense to and when there is time to.

Not saying that's what this is, but sometimes things bloat a tiny bit at a time.

"Hey Mr. Senior, I have a task to add fatigue stuff to the game, where should I put it?"
"Well, junior, I guess we could put it on CharacterStats since every creature in the game has a CharacterStats because every creature in the game can be killed, and if it can be killed, it will have fatigue. Yeah, put it there."


Yes, you could have created a separate fatigue class and system, but the bottom line is that CharacterStats would need it anyway, without exception.

Again, I am kind of just making up examples here.
 

mutonizer

Arcane
Patron
Joined
Sep 4, 2014
Messages
1,041
Yea, overall that's probably the difference between coding for pleasure, and coding for work: outside factors forcing to just focus on making things work as quickly as possible, instead of doing things the way you want them to. You keep putting up more and more shit in there in a rush, and end up with a monster that works...just looks weird.

I'm OCD as well on this kind of stuff, I go bonkers if I don't have my indents perfectly lined up, so really glad I don't have to do it professionally :)


edit: Hmm, your example just takes things that make the class concept work and I agree. But my point was for examples like this:

"Sir, I need to add the piercingDT modifier variable for our new piercing weapons system, where should I put it."
"No fucking clue man, what's open right now? CharactersStats? Put it there, it's fine..."
"And the lore status too?"
"Doesn't CharacterStats handles all creatures in the game already?"
"Yes it does sir but PC and companions will never need it, only some monsters, not even all of them."
"Nevermind that, put everything in there."
 
Last edited:

Sensuki

Arcane
Joined
Oct 26, 2012
Messages
9,800
Location
New North Korea
Codex 2014 Serpent in the Staglands Shadorwun: Hong Kong A Beautifully Desolate Campaign
I am probably the worst at programming in this discussion but I'll add my piece.

When I was looking at it I found some code that probably should have been deleted, there's code in there from the old armor system from early 2013 and there's old code on recovery time that's not used anymore, and there's old code on how they handle other things as well. Only reason I know this is because I know how the armor system works and I know how it used to work. I was snooping around to try and find the code that governed recovery time so I could see how to mod it but it's very confusing because there's a lot of stuff that isn't being used (well at least to me it looks like it isn't) so I haven't been able to find the exact methods in use yet. Same with applying minimum damage, there's code in there from the old armor system ... but the current way is supposed to be 10% of incoming damage (which I've tested and it works) but the old method that has 40% minimum and 50% crushing minimum is still there.
 

Immortal

Arcane
In My Safe Space
Joined
Sep 13, 2014
Messages
5,062
Location
Safe Space - Don't Bulli
Pretty Sure Anthony would mostly agree with this.. Curious about his opinion.. He is actually a game dev after all maybe the standards are different?

My Bullshit opinions:
Some code seems to be following a very strict concise pattern and some of it seems to be a little heavy on the in-lining.. Not sure how I feel about those Enums being shoved in the class namespace like that but honestly that class isn't that bad..

If you guys are actually guffawing at a 6000 line class.. I would argue you haven't worked on a large project and you have never had a strict deadline that was in jeopardy..
Upset about the patterns not being consistent? Your company must also have never hired contractors or new employees and never had a manager who was getting yelled at by another manager.. Oh and Unity doesn't exactly make things easier.

In a perfect world we want proper patterns and a consistent stack that is always modular and decoupled as much as possible.. The reality is.. Crunch times happen.. Requirements change and shit happens..

Anyone having trouble navigating that class is not a very strong developer.. Commented out code or code not being used in a BETA release that isn't even being deployed?! HOW DARE THEY! (I checked in code on Friday that many -redacted- customers are using right now.. There's tons of harmless code (mostly commented but not all) in that check in for future stories in our backlog that didn't make it in this sprint.. so what?)

Also if anyone is bitching about lack of code form or comments.. you do understand what decompilation does right? The fact that IL maintains class and method signatures should be praised.. you guys should check out some decompiled byteCode sometime.. :roll:

This is definitely true.

You might have a senior do the TDD and the initial implementation - someone like Tim Cain, Steve Weatherly, or Dan Spitzely. Their implementation is great. They move off to the next system and then changes or modifications to the system sometimes fall to a junior. The code will get reviewed of course, but a lot of the time you look at the change and why it was made, and then you look at your deadline and say, "That'll do, pig."

You know sometimes that when you are doing this that you are accumulating "code debt". In a perfect world, you will pay that debt off and get the system as clean and lean as you want. That doesn't always happen - and in this particular case, I'm still not seeing anything that CharacterStats wouldn't need.

Obviously as you gain experience, you also code systems in such a way where they can be easily changed and adapted (hopefully). The price of that adaptability is leanness.


Story time.
I have OCD. No surprise, many engineers do.

A long time ago, when I first started out - working for Enron, Dow Chemical, Xerox, Netscape, etc. I was always deleting code and making sure whatever I checked in was as clean as clean could be. My OCD demanded it of me.

As time went on, I realized I was constantly going back and changing things BECAUSE.... designs and specifications change, even in the non-gaming industry (the gaming industry is about 100x worse due to the nature of the product). It took awhile, but I eventually trained myself to comment out code, if def out code, or just remove the call and let the compiler no-op it.

I still hate looking at it - it keeps me riled up. Then I remind myself, that if they decide to change Sprint, or Fireball,or whatever example, I'm ready.


I actually argued the opposite direction when I first started looking at the AI code.. I posted a class diagram somewhere around here. I think they have a great pattern for adding AI to their AIManager Stack.. They make good use of state objects.. good use of abstraction.. The members I expect to be private or protected always are..

I actually have very little complaints about the PoE code.. Not sure what all the hubbub is about.. I've seen way worse.

I am probably the worst at programming in this discussion but I'll add my piece.

When I was looking at it I found some code that probably should have been deleted, there's code in there from the old armor system from early 2013 and there's old code on recovery time that's not used anymore, and there's old code on how they handle other things as well. Only reason I know this is because I know how the armor system works and I know how it used to work. I was snooping around to try and find the code that governed recovery time so I could see how to mod it but it's very confusing because there's a lot of stuff that isn't being used (well at least to me it looks like it isn't) so I haven't been able to find the exact methods in use yet. Same with applying minimum damage, there's code in there from the old armor system ... but the current way is supposed to be 10% of incoming damage (which I've tested and it works) but the old method that has 40% minimum and 50% crushing minimum is still there.

This is easy to say when you have tons of spare time and your on the outside looking in.. In some cases code should be deleted or refactored.. in some cases.. it causes more pain then it's worth..

Think about it from a developers perspective.. You've got Josh coming in with different ideas how things should work and is likely walking past your desk every day saying change this armor system to act like this.. change that feature to that and fix these bugs.. then next week he might come back and say.. "Nevermind I liked that old system but tweak it here"

What is that developer supposed to do? Back his code up to a weak old revision from the repository losing all the work he's done since then or worse.. trying to merge it and keep the build clean?

You can't possibly know what their dev cycles look like.. So don't judge it..



FYI This is a real world example of something that happened to Blizzard with World of Warcraft..


Greg Street (Lead Designer) was looking over abilities to prune for the hunter class and one of the dev's asked if they were keeping beast eyes or whatever (where you look through the eyes of your pet).. Greg said nah scrap it.. it's a confusing ability that doesn't offer much value.. just fun to mess around with.

The programmers clipped the code out and later their forum blew up with people mad about it's removal.. Greg replied telling them this very story about how the code was removed completely instead of commented out and now it would take a large amount of dev hours to put that code back in.. but they want to because they didn't expect people to miss it that much..
 
Last edited:

Immortal

Arcane
In My Safe Space
Joined
Sep 13, 2014
Messages
5,062
Location
Safe Space - Don't Bulli
I don't wanna beat a dead horse here guys.. but I wanna bring up one more thing about this class and why it is so top heavy on members... Unity generally assigns object representations to objects in the world space.. Like for unity your level editor and IDE are joined as one thing. The code you write directly affects the state of objects that the class you wrote represents or that extends that interface.

If you have level designers who aren't developers.. this makes their lives very easy for setting up the states of objects or building levels without needing to look at C# code.

EXAMPLE:

3XX0QhZ.png


So once again.. maybe that class isn't the be all end all of coding standards.. but as I said.. when it comes to Unity development.. Fuck it. :lol:
 

mutonizer

Arcane
Patron
Joined
Sep 4, 2014
Messages
1,041
Hmm, very true, unity does have it's....specifics and that would explain a lot, especially to allow designers to fine tune on the fly.
 

As an Amazon Associate, rpgcodex.net earns from qualifying purchases.
Back
Top Bottom