Skip to content

Issue 1099 dockingport spelling#1161

Merged
Dunbaratu merged 12 commits intoKSP-KOS:developfrom
hvacengi:issue-1099-dockingport_spelling
Oct 23, 2015
Merged

Issue 1099 dockingport spelling#1161
Dunbaratu merged 12 commits intoKSP-KOS:developfrom
hvacengi:issue-1099-dockingport_spelling

Conversation

@hvacengi
Copy link
Member

Fixes #1099

  • Corrects the spelling of acquire in the suffixes and throws a deprecation exception if called with the old spelling.
  • Added suffixes of NODEPOSITION and NODETYPE to make docking scripts a little easier and more accurate.
  • Also adds a HASMODULE suffix to PartValue, so to easily determine if the part has a specified module.

DockingPortValue.cs
* fix the spelling of the suffixes for acquire range, force, and torque.
* throw deprecation exception directing the user to use the correct
spelling if they use the old version.
DockingPortValue.cs
* add a suffix `NODEPOSITION` which returns the position of the actual
docking node itself, rather than the part's position
* add a suffix `NODETYPE` to allow you to differentiate between
different types of docking ports, in stock this is how you can tell the
"size" of the docking port.
PartValue.cs
* expose the shared objects as a protected variable so that classes that
inherit (like DockingPortValue) don't need to keep their own handle.
* add new suffix `HASMODULE` which can be used to determine if a part
has a specific module.
@hvacengi hvacengi added Not Ready Author is warning Reveiwer not to merge this PR yet. More edits are expected. documentation Change the Sphinx user documents. labels Sep 16, 2015
Copy link
Member

Choose a reason for hiding this comment

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

no non-private fields.

Copy link
Member Author

Choose a reason for hiding this comment

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

Access from an inherited class is a little bit of a hybrid issue. I agree in concept with not publicly exposing the member variable itself, and I don't think we want this variable publicly exposed anyways. Protected fields are only accessible to inherited classes, which makes it helpful to not hold a 2nd handle to the object. I think I've always treated protected fields similarly to private. Would you prefer that I implement a protected property, or just add the 2nd handle to the DockingPortValue class?

Copy link
Member

Choose a reason for hiding this comment

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

protected property with private setter would be fine.

@hvacengi hvacengi added Pending Documentation Author is warning Reviewer not to merge this PR yet. User doc changes aren't in the PR yet. and removed documentation Change the Sphinx user documents. labels Sep 22, 2015
PartValue.cs
* Moved the SharedObjects field `shared` to a protected property with a
private setter.
dockingport.rst
* Fix the spelling on the documentation of the acquire suffixes
* Add a warning identifying the breaking change.
@hvacengi hvacengi removed Not Ready Author is warning Reveiwer not to merge this PR yet. More edits are expected. Pending Documentation Author is warning Reviewer not to merge this PR yet. User doc changes aren't in the PR yet. labels Sep 29, 2015
@hvacengi
Copy link
Member Author

Assuming no further review questions pop up, this should be ready.

@hvacengi hvacengi added the Breaking Some user scripts that used to work will break (even if just in a minor way). label Oct 1, 2015
hvacengi and others added 2 commits October 13, 2015 16:24
PartValue.cs
* Apparently the implementation of HASMODULE did not get committed
correctly.  It usually helps if you actually change the copy/pasted
suffix code :-)
@erendrake erendrake assigned erendrake and Dunbaratu and unassigned erendrake Oct 20, 2015
Copy link
Member

Choose a reason for hiding this comment

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

These should be "0.18.0" not "18.0".

Copy link
Member Author

Choose a reason for hiding this comment

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

That's odd for me to make that kind of mistake. Do you want to make the change in you're review commit, or do you need me to make the change?

Copy link
Member

Choose a reason for hiding this comment

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

I won't be in a position to edit code until tomorrow morning. I can do it then or you can do it before if you like.

DockingPortValue.cs
* Update version to 0.18.0
PartValue.cs
* Remove old debug logging.
DockingPortValue.cs
* Remove another extra debug log.
@erendrake erendrake added this to the Pre 1.1 Release milestone Oct 22, 2015
Dunbaratu and others added 3 commits October 22, 2015 15:47
…g' into issue-1099-review

Strange merge conflict on pure whitespace line that made no sense
why it conflicted.  Had to manually change it.

Conflicts:
	src/kOS/Suffixed/Part/PartValue.cs
@hvacengi
Copy link
Member Author

@Dunbaratu should be good to go here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking Some user scripts that used to work will break (even if just in a minor way).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dockingport:aquire[force,range,torque] is a misspelling of "Acquire"

3 participants