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

Draw nano range on transport #4035

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Cheva998
Copy link

@Cheva998 Cheva998 commented Dec 18, 2024

Work done

This commit solves the issue #3912 - Have transport carrying conturret show buildrange on unload command
The new file draws a circle around a transport carrying a nano when its about to unload

Addresses Issue(s)

Test steps

To test there are 3 scenarios:

  1. A transport carrying a nano, when the unload command is executed, it shoud draw a circle on the ground with radius equal to the range of the nano
  2. A transport carrying any unit but a nano, when the unload command is executed it shouldn't draw the circle
  3. Several transports carrying units nanos and others than nanos, when the unload command is executed it should draw a circle on the ground with radius equal to the range of the nano

Screenshots:

BEFORE:

image

AFTER:

image

Draw a circle around a transport carrying a nano when its about to unload
This commit solves the issue beyond-all-reason#3912 Have transport carrying conturret show buildrange on unload command

To test there are 3 scenarios:
1. A transport carrying a nano, when the unload command is executed, it
   shoud draw a circle on the ground with radius equal to the range of
   the nano
2. A transport carrying any unit but a nano, when the unload command is
   executed it shouldn't draw the circle
3. Several transports carrying units nanos and others than nanos, when
   the unload command is executed it should draw a circle on the ground
   with radius equal to the range of the nano
Comment on lines +42 to +50
function MinValue(table)
local min = table[1]
for i = 2, #table do
if table[i] < min then
min = table[i]
end
end
return min
end
Copy link
Member

Choose a reason for hiding this comment

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

Try to use an existing common table function inside https://github.com/beyond-all-reason/Beyond-All-Reason/blob/master/common/tablefunctions.lua. If no suitable function exists, adds the new functionality there.

--------------------------------------------------------------------------------
local circleDivs = 96
local range
local isNanoTC = {}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
local isNanoTC = {}
local isImmobileBuilder = {}

Or isConstructionTurret or something similar. For code clarity, avoid using abbreviations, short forms, and slang.

local glLineWidth = gl.LineWidth
local glDrawGroundCircle = gl.DrawGroundCircle

for udid, ud in pairs(UnitDefs) do
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for udid, ud in pairs(UnitDefs) do
for unitDefId, unitDef in pairs(UnitDefs) do

Avoid short forms

--------------------------------------------------------------------------------
--vars
--------------------------------------------------------------------------------
local circleDivs = 96
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
local circleDivs = 96
local circleDivisions = 96

Or circleSegments or something similar

Copy link
Member

Choose a reason for hiding this comment

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

Change spaces -> tabs

Comment on lines +71 to +73
if transportWithNano[transportID] then
transportWithNano[transportID] = nil
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if transportWithNano[transportID] then
transportWithNano[transportID] = nil
end
transportWithNano[transportID] = nil

No need to check if value is not nil if just setting it to nil

Copy link
Member

Choose a reason for hiding this comment

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

What happens if the transport is destroyed while carrying the nano? Should also do a removal inside the UnitDestroyed callin.

@WatchTheFort
Copy link
Member

Tested it, looks great in-game. Could this be extended to transportable turrets as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants