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

POW Truck now ready to train. #574

Merged
merged 3 commits into from
Oct 31, 2021
Merged

POW Truck now ready to train. #574

merged 3 commits into from
Oct 31, 2021

Conversation

Jundiyy
Copy link
Collaborator

@Jundiyy Jundiyy commented Oct 17, 2021

POW Truck ready to be used.

End

Prerequisites
Object = TechWarFactory
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's 1 of 2 things.
Firstly a dummy object so units can't be abused with CommandSets and secondly, we will probably add this tech building as a fun tech for mappers to place on their maps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, so it doesn't exist yet, but will be added eventually. I don't see how exploiting command sets is a possibility here. Ordering to build a unit that has no button on remote machines just mismatches.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well the Tech building has been transferred from NProject but I didn't upload it yet, I thought I'd finish off unit codes first.
And the exploit would be possible because everyone using the patch would have the Command Button on their machines.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am in big favor of having new features not be exploitable by CommandSet.ini. Even original features for that matter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And the exploit would be possible because everyone using the patch would have the Command Button on their machines.

The game would still mismatch afaik, because the button does not exist on the commandset on the remote machines.

I am in big favor of having new features not be exploitable by CommandSet.ini. Even original features for that matter.

Me too. But this issue does not apply here, I think.

ButtonImage = SAPowTruck
ButtonBorderType = BUILD ; Identifier for the User as to what kind of button this is
DescriptLabel = CONTROLBAR:ToolTipChinaBuildSupplyTruck
End
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you put a newline at the end of the files? Or set up your text editor to do it for you? Works better with git, because per unix convention all lines are terminated with newline chars.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Judging by this change diff, originally there was no new line behind End as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure will do 👍 sorry, didn't know it would cause a problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Judging by this change diff, originally there was no new line behind End as well.

There was. It doesn't show as empty line on github though, because github is not a text editor.

@xezon
Copy link
Collaborator

xezon commented Oct 17, 2021

Comments for Patch104p are missing.

;BuildTime = 10.0 ;in seconds
;Buildable = No ;Can be built.
BuildCost = 500
BuildTime = 10.0 ;in seconds
Copy link
Collaborator

Choose a reason for hiding this comment

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

whitespace at end of line

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
BuildTime = 10.0 ;in seconds
BuildTime = 10.0 ;in seconds

@commy2
Copy link
Collaborator

commy2 commented Oct 18, 2021

As far as I can tell, all changes here could also be implemented in 1.04 via map.ini file.

@Jundiyy
Copy link
Collaborator Author

Jundiyy commented Oct 18, 2021

Added Patch104p and line breaks.

Comment on lines 2699 to 2702
;Behavior = ExperienceScalarUpgrade ModuleTag_10 ;Doesn't gain xp so no need.
;TriggeredBy = Upgrade_AmericaAdvancedTraining
;AddXPScalar = 1.0 ;Increases experience gained by an additional 100%
;End
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
;Behavior = ExperienceScalarUpgrade ModuleTag_10 ;Doesn't gain xp so no need.
;TriggeredBy = Upgrade_AmericaAdvancedTraining
;AddXPScalar = 1.0 ;Increases experience gained by an additional 100%
;End
;Behavior = ExperienceScalarUpgrade ModuleTag_10 ;Doesn't gain xp so no need.
; TriggeredBy = Upgrade_AmericaAdvancedTraining
; AddXPScalar = 1.0 ;Increases experience gained by an additional 100%
;End

@xezon xezon merged commit 1d01f77 into main Oct 31, 2021
@xezon xezon deleted the POWTruck branch October 31, 2021 18:05
commy2 added a commit that referenced this pull request Oct 31, 2021
xezon pushed a commit that referenced this pull request Aug 28, 2022
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.

3 participants