feat: Introduce a capability to limit the rotation of a turret. #5459

oo13 · 2020-10-18T11:01:50Z

Feature: This PR implements the feature request detailed and discussed in issue #3148.

MZ closed #3148 but, as far as I know, MZ and devs have not refused it.

Feature Details

This PR introduces a capability to limit the rotation of a turret, and it makes us possible to place the turrets in the arrangement of superfiring.

New syntax is similar to angled gun-port (#4801), and I owe this code to #4801, of course.

	# The turret is omnidirectional and points to the front when idling.
	turret x y outfit_name
	
	# The turret is omnidirectional and points to 'baseAngle' when idling.
	turret x y outfit_name
		angle baseAngle
	
	# The turret can rotate in the range from 'minAngle' to 'maxAngle' (clock orientation) and points to the center of the range when idling.
	turret x y outfit_name
		arc minAngle maxAngle
	
	# The turret can rotate in the range from 'minAngle' to 'maxAngle' (clock orientation) and points to 'baseAngle' when idling.
	turret x y outfit_name
		angle baseAngle
		arc minAngle maxAngle

And the turrets calculate the idle position considering the "parallel" parameter.

For Example:

	turret 0 -40 "Test Turret"
		angle -60
		arc -120 120
	turret 0 40 "Test Turret"
		angle 40
	turret 40 80 "Pug Anti-Missile"
		angle 180
		arc 90 180

A weapon also has an attribute "arc", which restricts the range where the turret can traverse. If the weapon is installed on a hardpoint with a limited arc, the narrower arc is used.

UI Screenshots

Current Version:

Noooo.

This PR:

Point to the front (same results in both versions):

Usage Examples

I provide this plugin test-for-turret-v6.zip and a savefile savefile-v6.zip.

Testing Done

Lock a ship and rotate my ship.
Deselect any ships and rotate my ship when there are some enemy ships in the system.
Select Opportunistic Tracking and fly a ship for a while.
Save and reload.
Set 'baseAngle' out of range from 'minAngle' to 'maxAngle' and ES outputs a warning message.
Install/uninstall/save/load weapons that has an attribute "arc".
Defend missiles with an anti-missile turret that is restricted arc.

Performance Impact

Slightly slow down because this PR adds some condition checks for all omnidirectional turrets.
And for the directivity turret AI.cpp checks the angle every expected target.
I think nearly all humans cannot feel the delay.

Zitchas · 2020-10-18T12:16:10Z

Thank you for making this PR. I haven't tested it yet, but I had been hoping for this and similar features to be implemented.

Potential uses:

Enables the creation of "partial use" turret slots that aren't as useful as full turret slots because they are limited to certain firing arcs (for instance, nose or tail gunners)
Enables more realistic directional restrictions for large ships such as battleships where turrets on one side may not be able to fire over the bulk of the ship at targets on the opposite side.
Moves us closer to fully supporting Escape Velocity, Escape Velocity Override, and Escape Velocity Nova ports; all of which had directionally limited turrets (and swivel canons)

From what I can see here, the restrictions are based on the turret slot. Would it be possible to have the restrictions placed on the outfit as well?

The reason I ask is because the Escape Velocity series had a few outfits that occupied an interesting space in terms of weaponry, such as the rear turret. This was effectively a blaster turret that was restricted to firing in a 90 degree arc centered on the rear of the ship. While its stats were basically identical to a regular turret of its type, it took up significantly less space as a trade-off for the fact that it didn't have the full traversal capabilities of a regular turret. This allowed it to be used on a few ships (such as the rapier, a heavy interceptor style ship) where mounting a full turret wouldn't have made much sense. As a general rule, the rear-turret was not a super effective weapon, but it did help the rapier deal with pursuing fighters, making it situationally quite valuable.

As such, I think it would make for an interesting pair of abilities to have both the ability to specify and limit firing arcs on the turret slot, as you have done here; as well as on outfits themselves, enabling specialized turrets designed for these restricted slots. Thus we would have a situation where any turret could be installed in the restricted turret slot, and work fine (just with the restricted arc); and we could also have turrets that don't have a full traversal that save some space or are cheaper or have other trade-offs that can be installed anywhere, but are intended for these restricted slots.

I don't think this additional ask should delay this particular PR; I would be more than happy to have these restricted turret slots merged now and the later version that enables a parallel outfit version to come later.

Thanks again!

oo13 · 2020-10-19T12:48:21Z

I would like to implement a weapon limited rotation, and the weapon specified a limited angle will be able to rotate right and left from the base angle as a center.

I don't realize any problem for the hardpoint that specifies an angles in this feature, but the default behavior is problematic.

Do you think what appropriate base angle is by default? If we install the turret specified a limitation on the existing ship, what direction the turret should point to? The front or outside?

In this PR now, the base angle will be equal to the hold position, that is the front of the ship. In the current ES, the turret point to the front of the ship when holding, but the base angle points outside from the center of the ship (although we can notice it only when departing a planet right after loading).

In my opinion, the turret should point to the front of the ship when holding because it uses for mining, but it's ridiculous that a hardpoint in the rear directs forward, or that a turret struggles to face the front of the ship when we install the turret specified a rotation limit to a turret mount that directs to backward.

Should the default base angle be outside only when a turret limiting rotation installs on it?

~~Ummm, coding in C++ is far easier than writing in English.~~

Zitchas · 2020-10-19T15:39:00Z

I think turrets should default to pointing forwards unless the restrictions on its arc do not allow that.

Thinking about what I said before, the weapon/outfit definition probably shouldn't generally specify a direction, but rather just an arc, which then combines with the turret hardpoint's direction.

Swivel canon example situations and how I think it should be handled: ("Forwards" and "backwards" mean "straight forward parallel to the ship's longitudinal axis" and "backwards" means straight backwards parallel to the ship's longitudinal axis")

Generic (unspecified direction) hardpoint: center of arc facing straight forward.
Hardpoint with specified direction: The weapon's arc should be centered on the specified direction.
- If that arc includes straight forward, then its resting position should be forward.
- If it does not include straight forward, but does include straight backwards, then it should point backwards.
- If it includes neither straight forward nor straight backward, then it should default to being centered on its arc.

This means that we don't need to have "nose turrets" or "tail turrets", but instead can simply have a turret pod, and whether they point forwards or backwards (or, for that matter, sideways as broadside turrets) is purely determined by the slot that they are installed in. And I know there have been a bunch of people interested in having broadside weaponry.

oo13 · 2020-10-20T11:24:57Z

OK, then, if we install a weapon with restricted arc on a default hardpoint, it is directed to the front.

But for hardpoint with specified direction, it should be what it specifies because this PR allows us to specify three parameters, the minimum and maximum angle of the arc, and the resting position. We also have a free to omit some parameters.

…lculate and to clone.

petervdmeer

Nice to see this feature being added. I do have some review comments about code readability/maintainability.
And if we want to have new hardpoint types in the future, then we should drive for commonality between guns and turrets also in PRs like this.

petervdmeer · 2020-10-22T20:01:57Z

source/Hardpoint.h

 	Angle baseAngle;
+	// Range of the capable angle to point to the target (turret only).
+	std::pair<Angle, Angle> movableAngle;


I think a name like turnRange will reflect the meaning of this variable better than movableAngle.

Similar for the comment:
// Range over which the turret can turn, from leftmost position to rightmost position (turret only)

I changed the name in local before reading your review. It's "angleOfTraverse" now.
(I already pushed it.)

Do you think which is name better?

Both turnRange as well as angleOfTraverse sound better to me than movableAngle. I'm not a native English speaker, so I leave it to other reviewers to comment on the most proper name.

source/Hardpoint.h

petervdmeer · 2020-10-22T20:13:12Z

source/Ship.cpp

+					{
+						const int end = min(4, grand.Size());
+						for(int i = 1; i < end; ++i)
+							angles.push_back(grand.Value(i));


I feel that a separate keyword for the turn-range will be more clear;
turnRange <min> <max> => for specifying the allowed turnrange for turrets (and maybe also for guns?)

"turnRange", "turn range" or "angle of traverse". I'll use what you like.
And I'll create "angle", "idle angle", "default angle", or something. I would like to know your suggestion.

My preference for the keyword is turn range, but I'm not a native English speaker, so you might want to get some input from other reviewers for this choice.

source/Ship.cpp

source/Angle.cpp

petervdmeer · 2020-10-22T20:48:00Z

source/AI.cpp

@@ -2720,7 +2720,8 @@ void AI::AimTurrets(const Ship &ship, Command &command, bool opportunistic) cons
 			{
 				// Get the index of this weapon.
 				int index = &hardpoint - &ship.Weapons().front();
-				double offset = (hardpoint.HarmonizedAngle() - hardpoint.GetAngle()).Degrees();
+				double offset = (hardpoint.GetBaseAngle() + hardpoint.HarmonizedAngle()


Do we really need to give a default where the turret aims at (and provide that as keyword)? (I would expect harmonized-forward if harmonized-forward is in range, otherwise normal forward if it is in range, and otherwise the center of the range).

An appearance of a ship depends on the idle position of turrets.
It can create a turret like the second and the third turret of this picture and to shoot an asteroid when mining as same as a current behavior. It points to the front, but possibly someone wants it points the back.

This can be a nice feature to have, but I'm not sure this is required now. We can also limit the rotation of a turret without providing a default holding/idle position.

source/Hardpoint.cpp

- Introduce an attribute "angle of traverse" for a hardpoint. - Change a type of parameter in the constructor of class "Hardpoint." - GetAngleOfTraverse() returns some valid value in case this is Omnidirectional or a gun port. - Rewrite some comments.

…cted hardpoint and weapon. The previous commit introduced the bug.

…tion.

ravenshining · 2020-11-21T23:25:42Z

"angle of traverse" seems a bit wordy. Just traverse would be kinder on those typing up content.

oo13 · 2020-11-22T12:11:17Z

Thanks.
So, the candidates are "turn range" and "traverse" now.

…x some style issues.

oo13 · 2020-12-27T13:46:31Z

I committed a "turn range" version for now.

Zitchas · 2021-01-01T04:42:56Z

Sounding good. How testable is this? Or, for that matter, how close is this to being usable?

oo13 · 2021-01-01T10:03:24Z

I expect this works fine if you can accept the name "turn range".

Zitchas · 2021-01-01T13:00:25Z

"turn range" is fine by me. "Angle of Traverse" is a technical gunnery and surveying term for almost exactly what we have here, so that term would be great, but I think "turn range" is sufficient and descriptive enough to convey the intended meaning.

edit for clarity: "Angle of Traverse" is, in my opinion, the most technically accurate label we could use. That being said, "turn range" is simpler and more easily understandable to the general population. (aka, to everyone who isn't a gunner or surveyor)

Reference: https://en.wikipedia.org/wiki/Gun_laying

Basically, the traverse is the horizontal angle the gun moves across. Can also be used as a verb "the gun was traversed to line it up with the next target." which means that it was moved horizontally to point at the new target.

tehhowch · 2021-01-01T19:09:05Z

I've not reviewed the code yet, but i think "swept angle" is probably a more accurate and expressive name for the keyword

Zitchas · 2021-01-01T21:49:56Z

Could you explain that term for me? All the examples I've been able to find online refer to wing shapes.

Since we're dealing with turrets, it seems appropriate to stick with already used terminology. That way if someone googles the term they'll be able to get information sources that describe what a traverse means for a weapon. (and guns in particular.)

tehhowch · 2021-01-01T22:37:36Z

Could you explain that term for me? All the examples I've been able to find online refer to wing shapes.

Since we're dealing with turrets, it seems appropriate to stick with already used terminology. That way if someone googles the term they'll be able to get information sources that describe what a traverse means for a weapon. (and guns in particular.)

The tool-tip I imagine for "swept angle" is "The angular range of protection provided by this weapon, in degrees."

If we worked in 3D, this quantity would be measured in steradians as we'd be working with solid angles, but we're in a simpler 2D world. As far as the etymology of the term, consider the behavior of the game's opportunistic turrets with no selected/hostile target in-system: they repeatedly turn back & forth over the same angular range relative to the ship's facing:

endless-sky/source/AI.cpp

Lines 2736 to 2738 in c5588f9

    
           // If there are no targets to aim at, opportunistic turrets should sweep 
        
           // back and forth at random, with the sweep centered on the "outward-facing" 
        
           // angle. Focused turrets should just point forward.

With this PR then, we allow content creators to implement custom "swept angles"

Zitchas · 2021-01-02T00:05:46Z

Thanks for the explanation.

Given that, I still think that "Angle of Traverse" makes more sense than "Swept Angle" because the traverse terminology already exists for shipboard weaponry, cannons, and turrets; whereas the "Swept Angle", is a more generic / mathematical term that is not generally used in the real-life equivalent fields.

That being said, this isn't a major issue for me. The former is a preference, in part based on the fact that I personally like it when my games teach me real-life terms and vocabulary where possible. If you are set on the alternative, though, I'm fine with it.

oo13 · 2021-01-02T10:29:54Z

I push a "swept angle" version now.

tehhowch

Another possible user-facing name for the functionality could be "angular coverage"

tehhowch · 2021-01-01T19:06:48Z

source/AI.cpp

+					const double targetDegree = (targetAngle - arc.first).AbsDegrees();
+					const double currentDegree = (hardpoint.GetAngle() - arc.first).AbsDegrees();


Should we use native Angle addition/subtraction here, instead of mapping an angle to the degree representation and then manipulating that?

targetDegree and currentDegree should be the representation of the degree, because offset may be a value in the range (-360, +360).
This is not an omnidirectional so it cannot traverse shortest path anytime.

It can be solved if the class Angle is able to convert the angle to a value in the range [0, 1) * 2**n, if you are concerned about the division of DEG_TO_STEP.

tehhowch · 2021-01-01T21:03:47Z

source/AI.cpp

+						const double degree1 = (range.first - angleToPoint).Degrees();
+						const double degree2 = (range.second - angleToPoint).Degrees();
+						if(fabs(degree1) < fabs(degree2))


Can we express this without converting both into degrees first?

It is needed because Degree() returns the nearer angle.

source/Hardpoint.h

source/Ship.cpp

source/AI.cpp

source/Hardpoint.cpp

Co-authored-by: tehhowch <tehhowch@users.noreply.github.com>

…ad of in-flight. And the turrets calculate the idle position considering the "parallel" parameter.

oo13 · 2021-01-20T13:47:28Z

I applied suggestions.

Also the idle position of the turrets is calculated when installing, instead of in-flight, and the turrets calculate the idle position considering the "parallel" parameter.

Amazinite · 2021-01-21T19:56:40Z

I know the keyword has gone through a number of changes already, but why not just "arc"? Turrets would then have a "(facing) angle" and "arc (of fire)" or "(firing) arc." "Swept angle" seems wordy/clunky to me. "Arc" makes more sense to me, since we are quite literally defining an arc of a circle within which the turret can fire. (People have even been using "arc" to refer to this feature, and various points in the code refer to it as an arc.)

oo13 · 2021-01-22T09:56:37Z

I post the "arc" version now.

EjoThims · 2022-05-30T09:01:16Z

How's it looking for this one to move forward?

Zitchas · 2022-05-30T12:37:17Z

Unclear. There's conflict(s) to resolve, and debate about the terminology to use, and a few comments from Tehhowch that need to be resolved. I'm not enough of a coder to be able to estimate how quickly this could be brought up to mergeable state.

For the terminology: three options suggested:

"Angular Coverage" : Descriptive term that basically explains itself clearly.
"Arc" : Simple and to the point, and commonly used English term for a portion of a circle.
"Angle of Traverse" : This is the proper terminology used in artillery and shipboard weaponry to describe this exact thing we are discussing.
"swept angle" : This is the proper mathematical term to describe the span of the arc covered.

My personal preference is for the "angle of traverse." If there is a real-life clearly defined term for what we are trying to describe, I feel that we should use it. It's relatively clear what it means, and has a completely unambiguous and precise real-world definition for those that know it, easily searchable for more information, and even encourages people to learn more proper real-world vocabulary, which I also feel is a desirable thing.

EjoThims · 2022-05-30T13:02:51Z

I think leaving it as "arc" after that was implemented for Amazinite's suggestion is fine, but seems the comments from Tehhowch need follow up from Teh.

Either way, now that it's bumped, will be easier to figure out if it's candidate for dreaded "waiting on OP" label.

oo13 · 2022-05-30T13:07:24Z

I'm sorry for unclear state of my PRs.
You can take over any PRs initiated by me. I would answer your questions, but I would neither update nor test the PRs. Should I close them?

EjoThims · 2022-05-30T13:15:04Z

I'm sorry for unclear state of my PRs. You can take over any PRs initiated by me. I would answer your questions, but I would neither update nor test the PRs. Should I close them?

Sad to hear. :(

That decision should come down from others like Amazinite and Tehhowch, though, and will probably be made based on what they can readily make mergeable and what would continue to sit.

Probably best to go ahead and toss the same comment on any other outstanding PRs you have though, so we know which are in that state and to get them that attention.

oo13 · 2022-08-05T13:08:25Z

Thank you, 1010todd!

Introduce a capability to limit the rotation of a turret.

3ec8e09

Amazinite added the enhancement A suggestion for new content or functionality that requires code changes label Oct 18, 2020

oo13 added 2 commits October 21, 2020 21:59

Refactoring: some names are changed.

b4bf606

Refactoring: preserve the angles parameter of the constructor to reca…

653cba8

…lculate and to clone.

petervdmeer requested changes Oct 22, 2020

View reviewed changes

oo13 added 5 commits October 23, 2020 19:13

The weapon has the "angle of traverse" attribute now.

d854fd8

A parameter "angle of traverse" affects an anti-missile turret now.

480418f

Apply some feedbacks.

21779e3

- Introduce an attribute "angle of traverse" for a hardpoint. - Change a type of parameter in the constructor of class "Hardpoint." - GetAngleOfTraverse() returns some valid value in case this is Omnidirectional or a gun port. - Rewrite some comments.

Fix a bug where causes a stuck turret when combining with an unrestri…

097c174

…cted hardpoint and weapon. The previous commit introduced the bug.

Fix a bug where a restricted turret might not return to the idle posi…

e132fbb

…tion.

Change the parameter name "angle of traverse" to "turn range", and fi…

743895c

…x some style issues.

tehhowch self-requested a review January 1, 2021 05:28

Change the attribute name "turn range" to "swept angle".

2869ea0

tehhowch reviewed Jan 18, 2021

View reviewed changes

Apply suggestions from code review

5908eb1

Co-authored-by: tehhowch <tehhowch@users.noreply.github.com>

oo13 added 7 commits January 20, 2021 20:05

Apply a suggestion by tehhowch.

96e741b

Refactor: Add Angle::IsInRange(pair<Angle, Angle>).

8c8d4f6

Refactor: Rewrite some comments.

6c9584d

Refactor: Rename the struct AnglesParameter to BaseAttributes.

1cd3460

Avoid incorrect warning message.

8b5fd8c

The idle position of the turrets is calculated when installing, inste…

1d069ed

…ad of in-flight. And the turrets calculate the idle position considering the "parallel" parameter.

Fix a wrong message of warning.

c8f57e3

Amazinite added this to the 0.9.13 milestone Jan 20, 2021

Change the attribute name "swept angle" to "arc".

0e4dff3

Amazinite removed this from the 0.9.13 milestone Feb 13, 2021

1010todd mentioned this pull request May 25, 2021

Swivel guns #6006

Closed

Zitchas mentioned this pull request Jun 18, 2021

Economy Test #6050

Closed

oo13 added 2 commits July 21, 2021 19:25

Change the tip's entry "swept angle:" to "arc:".

d1b63d5

Merge branch 'master' into limited_turret_turning_angle

8811090

captain0xff mentioned this pull request Jun 13, 2022

Added option for firing and rotating the ship with mouse #6936

Merged

1010todd mentioned this pull request Aug 4, 2022

feat: Ability to limit turret firing arc #7094

Merged

10 tasks

oo13 closed this Aug 5, 2022

1010todd mentioned this pull request Apr 26, 2023

Limited turret arc endless-sky/Endless-Sky-Delta#18

Merged

		const double targetDegree = (targetAngle - arc.first).AbsDegrees();
		const double currentDegree = (hardpoint.GetAngle() - arc.first).AbsDegrees();

feat: Introduce a capability to limit the rotation of a turret. #5459

feat: Introduce a capability to limit the rotation of a turret. #5459

Conversation

oo13 commented Oct 18, 2020 • edited Loading

Feature Details

UI Screenshots

Usage Examples

Testing Done

Performance Impact

Zitchas commented Oct 18, 2020

oo13 commented Oct 19, 2020

Zitchas commented Oct 19, 2020

oo13 commented Oct 20, 2020

petervdmeer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oo13 Oct 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oo13 Oct 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oo13 Oct 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravenshining commented Nov 21, 2020

oo13 commented Nov 22, 2020

oo13 commented Dec 27, 2020

Zitchas commented Jan 1, 2021

oo13 commented Jan 1, 2021

Zitchas commented Jan 1, 2021 • edited Loading

tehhowch commented Jan 1, 2021

Zitchas commented Jan 1, 2021

tehhowch commented Jan 1, 2021

Zitchas commented Jan 2, 2021

oo13 commented Jan 2, 2021

tehhowch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oo13 commented Jan 20, 2021

Amazinite commented Jan 21, 2021 • edited Loading

oo13 commented Jan 22, 2021

EjoThims commented May 30, 2022

Zitchas commented May 30, 2022 • edited Loading

EjoThims commented May 30, 2022

oo13 commented May 30, 2022

EjoThims commented May 30, 2022

oo13 commented Aug 5, 2022

oo13 commented Oct 18, 2020 •

edited

Loading

oo13 Oct 23, 2020 •

edited

Loading

oo13 Oct 23, 2020 •

edited

Loading

oo13 Oct 23, 2020 •

edited

Loading

Zitchas commented Jan 1, 2021 •

edited

Loading

Amazinite commented Jan 21, 2021 •

edited

Loading

Zitchas commented May 30, 2022 •

edited

Loading