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

Chopper support #2712

Closed
wants to merge 58 commits into from
Closed

Chopper support #2712

wants to merge 58 commits into from

Conversation

pops64
Copy link
Contributor

@pops64 pops64 commented Aug 17, 2023

Currently 1 Tipper is supported. Correctly selects non fruit side and turns when chopper is finished. Looking for testers as there are lots of edges cases need to be done.
Things to test as this is almost complete

  • Combines - Nothing Changed code was moved around
  • Headlands - Sharp turns or tight turns seems to handle rounded turns just fine
  • Sugarcane - Should work but I haven't coded or tested anything. Small work width might cause issues
  • AD Interactions Both Chaff and Sugarcane
  • Grass - Haven't tested, this might perform poorly due grass not being a fruit
  • Calling a 2nd tipper when current is almost full. This might require some effort but I would like to get it done before finalizing

@pops64
Copy link
Contributor Author

pops64 commented Aug 18, 2023

Does not support the one chaff auger. First guess is the tip side setting something to look int

@Tensuko
Copy link
Contributor

Tensuko commented Aug 18, 2023

About adding translations, pls follow our Wiki Guide.
https://github.com/Courseplay/Courseplay_FS22/wiki/Translations

@Tensuko
Copy link
Contributor

Tensuko commented Aug 18, 2023

From my 1st test, the UI seems to fit, but I already have a few things that needs to be changed/added.

Pls remove the info msg for chopper.
info msg

Use the fruit check area to see on what side the unloader needs to drive. Also on the 1st headland, the unloader have to drive behind the chopper.
fruit check

Make sure the distance is not that high, seems like your pipe offset is not correct calculated.
distance

To solve traffic conflicts, check the combine unloader and implement the same behavior.
traffic

@@ -17,10 +17,12 @@
<Values>
<Value name="UNLOAD_COMBINE">1</Value>
<Value name="UNLOAD_SILO_LOADER">2</Value>
<Value name="UNLOAD_CHOPPER">3</Value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big fan of adding another option here, as this complicates the usage for the player.
Maybe instead of creating a new unload copper strategy, we could keep the combine one for both but
add a context with dependcy injection depending on the found unload target(combine or chopper)?

Copy link
Contributor

Choose a reason for hiding this comment

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

instead of creating a new unload copper strategy, we could keep the combine one for both

I get why you don't like that but I still think it would be cleaner to have separate strategies. Another option could be to create a strategy for idle unloaders and instantiate the combine or chopper strategy only after the unloader is called by the combine/chopper.

Copy link
Contributor Author

@pops64 pops64 Aug 18, 2023

Choose a reason for hiding this comment

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

I would go further and move the self unloading code out as well, just my two cents tho. You would be adding a lot of if statements to combine if you combined them as there are some things you just don't want a chopper unload driver to do

@@ -0,0 +1,60 @@
---@class ChopperController : ChopperController

Copy link
Contributor

@schwiti6190 schwiti6190 Aug 18, 2023

Choose a reason for hiding this comment

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

This controller doesn't follow the convention of the other implement controllers, which are based on Spec's or external mods. Giants doesn't differentiate between combine or chopper, so both use a common combine controller for the common combine specialization. Unless you've got anything special which can be put here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only did it to give clear separation between Chopper specific functions that CP team made to fix issues and the combine specific stuff. And as you said the combine stuff is related so I made sure to inherit the combine stuff just in case there was combine stuff used

@@ -124,59 +123,4 @@ end
function CombineController:isDroppingStrawSwath()
return self.combineSpec.strawPSenabled
end
-------------------------------------------------------------
Copy link
Contributor

@schwiti6190 schwiti6190 Aug 18, 2023

Choose a reason for hiding this comment

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

See the comment above about the chopper controller.
This logic below works mostly indirect with the combine Spec and should be placed here.

@pops64
Copy link
Contributor Author

pops64 commented Aug 18, 2023

I will have to test for headlands. Currently relying on the pipe in fruit map for functionality. The fruit area is unreliable until your in the fruit. I am using the max discharge distance to calculate the pipe offset. The conflict in headland turn was an edge case I was worried about why I prefer to always drive on the not fruit side . I will have to make a special handle for headland turns. I will look at changing the text. I know the UI is a beast onto itself why I don't like to go tinkering with it.

I will fix the translations used to the manual way from FS17 days

@Tensuko
Copy link
Contributor

Tensuko commented Aug 18, 2023

You should stop compare everything with FS17 and have a closer look at FS19 where the headland turn and traffic conflicts and side of chopper are all handeled already....

Reduced the pipeoffset by 10%. Reduced the pipeoffset by minus 2 when calculating pipe in fruit for better reliability. Overhauled the chopper drive away course
@pops64
Copy link
Contributor Author

pops64 commented Aug 18, 2023

@Tensuko I didn't play in 19 so I have no point of reference. The combine/chopper code is always a beast to look at.

I made some tweaks. The max discharge distance of a chopper is 20 or something meters and you have a small working width which is why it looks off. I looked into the info message and it would be a pain to fix/replace this. The headland turn I will have to think of a fix. I noticed that sometimes the unload driver reaches the end of the row before the chopper causing the issue. I will think of a solution

may cause a nil error, also updated naming convention
More reliable method of determine fruit side as been implemented. Still limited due to look ahead and if the field edge is at an extreme angle to the chopper
Fixed the fruit side check again. Improved the Chopper unload turn. Cleaned up and reorder the code some
Call stacks
Edge Case where it won't release the current driver.
Some edge cases to figure out. End of rows causes issues. Partially due to reliance on pipe in fruit map which isn't accurate for choppers. Need to create a better method for getting the chopper to send the tipper home as the current method resets after 3 secs which causes issues if the tipper is blocked on its way home
changed from isManauvering to isTurning for checking if the chopper is turning around. is Manauvering was causing false positives because isManauvering also checks unload state in a or condition possible bug
The marker node creation uses DirectionNode. When we calculated the back distance we used the root node. Not all vehicles have the root and direction node in the same place
The problem was in marker creation
This reverts commit 1826d4c.
Replaced combineRootNode to AI Direction in drive besidesCombine as we are now measure pipeoffsetZ from the Direction node and not the root. This will cause misalignments. Removed the minus 2 as well as this was probably a fix to correct this issue. Move the calculation of the pipeOffset to its separate function. For choppers we want the pipeBaseNode as our reference as this point is fixed

Two known call stacks exist. End of chopper course doesn't exit cleanly. A combineToUnload is nil in idle can't find where we are calling this from an idle state
Also fixed the unload driver in range function
Noticed a state where it couldn't decide whether to go or not. I am guessing it was the changing unloader logic as we now currently check to ensure we are only unloading to our current unloader in the unload state or a human/AD before driving.

Also another attempt at fixing the end of work call stack. It as to do with looking at waypoints beyond the end of the course. Made a new function in course to provide this check
@pops64
Copy link
Contributor Author

pops64 commented Aug 30, 2023

So I am very close to finalizing this. I just need some testers to put it through its paces. There is one end of work bug. I think I have squashed it. There is also a combineToUnload being called when nil. It isn't game breaking but generates that ugly block of log text once. I think it as to do with checkRendevous but the log isn't providing any hints to where it is at.

Let me know if there are oddities or rough spots still left. I know the turns are still large but it is using pathfinder so if there are any obstacles it will avoid them as best as pathfinder can do

@coatsy35
Copy link

coatsy35 commented Sep 3, 2023

So I am very close to finalizing this. I just need some testers to put it through its paces. There is one end of work bug. I think I have squashed it. There is also a combineToUnload being called when nil. It isn't game breaking but generates that ugly block of log text once. I think it as to do with checkRendevous but the log isn't providing any hints to where it is at.

Let me know if there are oddities or rough spots still left. I know the turns are still large but it is using pathfinder so if there are any obstacles it will avoid them as best as pathfinder can do

I can help test this if you want.

@regs1980
Copy link

regs1980 commented Sep 4, 2023

I can help test this if you want.

Test and write it here your experience
#2701

Will only drive 25m when on a headland. Will also check to see if the wide 180 will put the driver off the field
@Tensuko
Copy link
Contributor

Tensuko commented Sep 5, 2023

#2701 (comment)

@Tensuko Tensuko closed this Sep 5, 2023
@coatsy35
Copy link

coatsy35 commented Sep 5, 2023

That's a shame. CP is in need of chopper mode. Just about to go into the phase of my farm, where I'm going to have a Biogas plant and large cow sheds, so would be chopping a lot. I also run a ton of machine either in one field or on the map. Last night I had 10 different fields with something happening in them automated, most of the had multiple combines and trailers, some drilling, some applying fertiliser all working at once and all controlled by CP and AD. Guess I'll be using AD for chopping again. @pops64 is it impossible for your vision to adhere to the CP coding standards. Lots of people would benefit from your code and knowledge on chopping I'm sure. The chopper presumably would have been in a separate module, so it was not confused with harvesting etc...

@Tensuko
Copy link
Contributor

Tensuko commented Sep 5, 2023

We are not in need of a chopper mode.
Nothing is that urgent to have an uncomplete and "not so well" working mode just to rush something.

@Tensuko
Copy link
Contributor

Tensuko commented Sep 5, 2023

Addition: This is not a "there will never be a chopper mode".
But not right now, when there is time for it, Our priorities right now are on other projects as you might know.

@coatsy35
Copy link

coatsy35 commented Sep 5, 2023

Addition: This is not a "there will never be a chopper mode". But not right now, when there is time for it, Our priorities right now are on other projects as you might know.

I understand. I'm mostly looking forward to the day when we have new course generator. It looks ace from the screenshots I've seen 😍

@pops64
Copy link
Contributor Author

pops64 commented Sep 5, 2023

I will leave my fork open. Please post any game breaking bugs there. Any further development has been halted but I will fix crashes and keep it updated with the main branch. https://github.com/pops64/Courseplay_FS22/tree/ChopperSupport

@Courseplay Courseplay locked and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants