Jump to content

Remove Surgery Menu for Autopsy


Shadeykins

Recommended Posts

9 hours ago, IK3I said:

Snowflaking code actually refers to putting something in that requires hooks and checks from a bunch of unrelated things to make it work. Like the pod code is slightly snowflakey in that it has to have it's own subset in the main damage and AI functions rather than independent ones as an example. In other words, Snowflaking is just another word for poor coding practices that act against OOP, the route that this codebase has decided to fully embrace, and ultimately lead to inconsistency and spaghetti code.

 

That being said, it is commonly used as a cringy catchall for content that requires changes that feel different in game far more than they are in code. In this case, when people say snowflaky, it would be fair to say that it's the feel of it rather than the real details of the implementation. while this is ultimately valid criticism, it's being attributed to the wrong thing and using the wrong term in the wrong context to boot. 

Second part was my main point here since proposed implementation was a variable check in one script that's already proc'd when the attackby on the scalpel fires; subsequently this required nothing to be cross-referenced which is why LTH calling it massive snowflaking threw me off. Interdependency in scripts isn't a bad thing and helps to make code more lightweight (you should always strive for as few functions as possible), but it does come with the caveat of less modularity. Again it still seems like a stylistic preferenc, as with adequate tabbing and proper commenting it's a non-issue, I've also always been of the "less is more" camp--which the proposed PR does an even greater job of so no complaints in that area.

I seem to recall having a tool that labelled and categorized scripts at one point, and told you which ones had dependency or co-dependency with which.

7 hours ago, IK3I said:

On a side note, can confirm, learned C++ first, I fully understand that joke. Halp me.

People who put closing brackets at the end of a line instead of on a new line deserve death. >:C

Also, I'm pretty sure Purpose was responding to another post, not yours in particular.

Edited by Shadeykins
Link to comment
Share on other sites

I think people are reading way too much into the whole 'snowflake' thing. The initial suggestion involved creating an autopsy table and a bunch of special rules that are inconsistent with the way surgery normally works. That is something the codebase heavily frowns upon unless absolutely necessary (such as IPCs).

Taking surgery out of the equation entirely was a clever solution to the problem that satisfies all parties. Pretty cut and dry, everyone's happy now.

Edited by Love-To-Hug
Link to comment
Share on other sites

22 hours ago, Love-To-Hug said:

I think people are reading way too much into the whole 'snowflake' thing. The initial suggestion involved creating an autopsy table and a bunch of special rules that are inconsistent with the way surgery normally works. That is something the codebase heavily frowns upon unless absolutely necessary (such as IPCs).

Taking surgery out of the equation entirely was a clever solution to the problem that satisfies all parties. Pretty cut and dry, everyone's happy now.

As the person who made the original suggestion, no. I brought it up as an option, but stated it's not a requirement because the map already references the table in the morgue as autopsy_table. I'm pretty sure I have the ultimate authority (as the person who made the suggestion) to call you out on deliberately misrepresenting what the suggestion is.

It's literally a variable check, and I'd appreciate it if you stopped being ridiculous about it.

Link to comment
Share on other sites

6 hours ago, Shadeykins said:

As the person who made the original suggestion, no. I brought it up as an option, but stated it's not a requirement because the map already references the table in the morgue as autopsy_table. I'm pretty sure I have the ultimate authority (as the person who made the suggestion) to call you out on deliberately misrepresenting what the suggestion is.

It's literally a variable check, and I'd appreciate it if you stopped being ridiculous about it.

Creating an exception for autopsy as a surgery operation is snowflaking the code by the maintainers' own definition, which is why the solution to this problem was removing it as a surgery operation.

I'm honestly not sure why this is such a big deal. A solution was found, so we should probably just move on.

Edited by Love-To-Hug
Link to comment
Share on other sites

On 2017-03-23 at 9:10 AM, Love-To-Hug said:

I really hate this suggestion, I'll be honest with you. This is major code snowflaking (and with that comes bugs) just to save two clicks.

This is the entire reason I keep beating a dead horse, because as I've explained (both via explanation and by literally showing you what this sort of code would look like in a standard, easy to parse programming language) this is a simple variable check. in BYOND terms, it's performing a check after the attackby to look at the table, which judging by the fact that failure %'s are in place prior to conducting surgery, already happens prior to the menu being called.

On 2017-03-27 at 8:59 PM, IK3I said:

That being said, it is commonly used as a cringy catchall for content that requires changes that feel different in game far more than they are in code. In this case, when people say snowflaky, it would be fair to say that it's the feel of it rather than the real details of the implementation. while this is ultimately valid criticism, it's being attributed to the wrong thing and using the wrong term in the wrong context to boot.

Despite that you've persisted on this line of logic that this is somehow "major code snowflaking" and the argument that my suggestion would somehow create instability/undue work/pressures on coders/maintainers. Then, when I respond to your assertions, you say I'm being "remarkably condescending".

Yes, a QoL PR got pushed through on the GitHub (or not yet merged, rather) which is great--I'm genuinely happy about this because it means autopsies won't make my wrist hurt. Actively trying to snub my suggestions for what seems to be no good reason, not so much. I'm not sure why you think I wouldn't be prompted to respond to you.

Link to comment
Share on other sites

23 minutes ago, Shadeykins said:

Despite that you've persisted on this line of logic that this is somehow "major code snowflaking" and the argument that my suggestion would somehow create instability/undue work/pressures on coders/maintainers. Then, when I respond to your assertions, you say I'm being "remarkably condescending".

Responding to my criticism by telling me to take a programming course is what I'd consider to be condescending, yes.

I think there is a major disconnect here. At no point was it my intention to imply that your suggestion would be difficult to implement. It's just the way in which you wanted to do it is not generally considered acceptable practice by the Paradise codebase. The idea behind it - making autopsy less tedious - is not anything I was ever against. Removing it as a surgical procedure is something neither of us thought of, and it's a very elegant solution.

This is not saying anything about you as a person or your programming skill. I don't really understand why this has to be so personal.

Edited by Love-To-Hug
Link to comment
Share on other sites

On 2017-03-23 at 9:10 AM, Love-To-Hug said:

I really hate this suggestion, I'll be honest with you. This is major code snowflaking (and with that comes bugs) just to save two clicks.

It's just the way in which you wanted to do it is not generally considered acceptable practice by the Paradise codebase.

 

Here are the Paradise GitHub's standards, because Paradise doesn't actually have explicit code contributing standards/best practices.

* Do not push directly to the repository, always make a pull request. (Nothing to do with suggestion).

* Your pull requests should be atomic. Balance changes, bug fixes and new features should all be in separate pull requests. (Nothing to do with suggestion).

* Your commits should be atomic. Make one commit for each distinct change, so if a part of a PR need to be removed/change, you can simply modify that single commit. (Nothing to do with suggestion).

* Please document and explain your pull requests thoroughly. What each commit changes and why. We do not want to have to read all of your commit names to figure out what your pull request is about. (Nothing to do with suggestion).

* Make sure to use the mapmerge tools on all map edits to the primary map files. Pull requests that contain edits that don't use the mapmerge tool will be automatically denied, unless explicit permission was given. (Nothing to do with suggestion).

* Any pull request that is not solely composed of fixes or non gameplay-affecting refactors must have a changelog. See here for further instructions, and here for an example changelog. Alternatively, inline changelogs are supported through the format descriped here. (Nothing to do with suggestion).

* Any type and proc paths MUST use absolute pathing unless the file you are working in primarily utilizes relative pathing.

Before you even attempt to erroneously bring this up, here's what's meant by pathing.

https://nanotrasen.se/wiki/index.php/Code_Contributing_Standards

This is from an abandoned wiki page that was established over a year ago when the idea for having code contributing standards first popped up. The idea never took off, and Paradise has never had "code contributing standards"--the original thread still exists under the wiki development subforum. The only "standards" for the codebase are that your code compiles, you're using the efficient calls as to not cause undue stress on the server, and that a maintainer approves it. I also genuinely fail to see how something that hasn't even been coded can be in violation of non-existent coding standards.

and last but not least...

* PRs should not have any merge commits except in the case of fixing merge conflicts for an existing PR. Use git rebase or git reset to update your branches, not git pull. (Nothing to do with suggestion).

Please tell me where my non-existent code violates the GitHub guidelines, or the non-existent guidelines for the Paradise codebase. Maybe then, once this mystery has been solved, I can appreciate why you seem to randomly hate a suggestion for what seems to be absolutely no reason at all.

Link to comment
Share on other sites

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue. Terms of Use