KSP-RO / ProceduralParts

A continuation of StretchySRBs, which is a continuation of StretchyTanks
71 stars 79 forks source link

KSP 1.0 #121

Closed NathanKell closed 9 years ago

NathanKell commented 9 years ago

KSPAPIExtensions is updated (by @taniwha-qf ) and on the repo, so that should work. I haven't tried compiling PP in 1.0 yet, however.

Certainly ProceduralSRB will need a bit of work now that thermal is so vastly different.

OtherBarry commented 9 years ago

How easy would proc heatshields (for stock thermal) be?

On Tue, Apr 28, 2015 at 1:13 PM, NathanKell notifications@github.com wrote:

KSPAPIExtensions is updated (by @taniwha-qf https://github.com/taniwha-qf ) and on the repo, so that should work. I haven't tried compiling PP in 1.0 yet, however.

Certainly ProceduralSRB will need a bit of work now that thermal is so vastly different.

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121.

BenChung commented 9 years ago

ProceduralSRB is a bit more broken than that - right now, it won't work at all. What's happening is srbConfigsSerialized is vanishing when the part is instantiated into the editor, so the SRBs that are added into the scene promptly NPE out and start belching flame and generally not working. Is this a problem in the mod, or in Unity/KSP?

BenChung commented 9 years ago

@OtherBarry It doesn't seem to difficult - the module for heatshields is fairly simple. Balancing it is going to be rather more difficult, however, as the stock heatshields do not appear to have ablative material scale with volume. I have this mostly done in my pull request - but it's still rather imbalanced (1m balanced, 2.5 and 3.75 much less so).

OtherBarry commented 9 years ago

Ah yeah. Similar issue with the DRE heatshields. I believe it was fixed by going by surface area, which i hope is what the stock ones scale by.

On Tue, Apr 28, 2015 at 2:28 PM, Benjamin Chung notifications@github.com wrote:

@OtherBarry https://github.com/OtherBarry It doesn't seem to difficult

  • the module for heatshields is fairly simple. Balancing it is going to be rather more difficult, however, as the stock heatshields do not appear to have ablative material scale with volume. I have this mostly done in my pull request - but it's still rather imbalanced (1m balanced, 2.5 and 3.75 much less so).

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-96905729 .

BenChung commented 9 years ago

Yes, that's exactly what's happening, I just don't know how to make it work with the cfg file (it isn't obvious from the DRE ones).

NathanKell commented 9 years ago

Stock heat shields scale ablator by surface area rather than volume, and same for mass. Since the heat flux is based on surface area, that seemed sensible.

As for the cloning--serialization was turned off for ConfigNode in 1.0, so alternate methods (like pulling from symmetrycounterpart or partprefab, if we're null) need to be used.

BenChung commented 9 years ago

@NathanKell Something's still a bit weird. part.partInfo.partPrefab is null when OnAwake is called on the thumbnail object, and it refers to this when the object is added to the scene, and at this point SymmetryCounterpart is still empty (for obvious reasons). Do you have any other suggestions as to how to persist the bell data?

ItMustBeACamel commented 9 years ago

Has anyone found out out how the new aero works? Seems like KSP calculates the drag values once a part gets loaded and then stores them into a config file. Do we have to recalculate drag after changing shape?

BenChung commented 9 years ago

@NathanKell The only real solution that I can come up with is to either put the SRB bell data into a singleton a la RealChute, or to jam it into a private static variable within the class. partPrefab is just returning the (literal) self object, and I can't seem to find any other way of getting at the original data.

BenChung commented 9 years ago

On my fork, I have a version that has fixed SRBs (I just put in a static field, sorry), and I also have a preliminary fix for the floating parent/children after reload (though the SRB bells seem to mess it up for some reason).

Edit to add: someone who understands the SRBs more than I do is going to have to fix them. The system isn't choosing the right attachment node, clearly, but I have no idea how to fix it.

@ItMustBeACamel I'm going to be looking at aero next - I'm not sure at this point.

BenChung commented 9 years ago

@ItMustBeACamel Yes, we need to update the aerodynamic model as we go - look at DragCubes. I have to stop now - be back later today.

NathanKell commented 9 years ago

You'll want to look at ModuleProceduralFairings for how to render procedural dragcubes as needed. Regarding attach nodes, their attach orientations have to be correct: 0,1,0 for top nodes, 0,-1,0 for bottom nodes, etc.

BenChung commented 9 years ago

You'll want to look at ModuleProceduralFairings for how to render procedural dragcubes as needed.

Yes, that's what I found too.

Regarding attach nodes, their attach orientations have to be correct: 0,1,0 for top nodes, 0,-1,0 for bottom nodes, etc.

They are - that code is unchanged (and it isn't the player attaching things - it's the code that reattaches them when you reload the ship from save). The problem is that ProceduralSRB is changing the attach points after ProceduralPart has made them (as far as I can tell), causing the child parts to become detached (though I'm not entirely sure why). One solution would be to have a mechanism for ProceduralSRB to delay ProceduralPart attaching child elements, but that seems a bit ugly.

If you have the time, my fork demonstrates the bug nicely. Just attach something on the bottom of an SRB, then reload the file and note what happens.

ItMustBeACamel commented 9 years ago

Well, that was easier than expected. :D Thanks for the tip. I built one rocket with a procedural cone at the top and another, identical one, but with the cone upside down and the pointy one reaches an altitude nearly twice as high than the other one. So I assume its working.

BenChung commented 9 years ago

The new problem of the day appears to be that the SRB thrust isn't changing when the slider is altered. As far as I can tell, the root cause is that Engine.maxThrust and the engine module neither actually alter output thrust when the maxThrust field is changed, but this seems insane. Did anything in engines change?

NathanKell commented 9 years ago

maxthrust is only used to calculate maxFuelFlow now. Change that.

BenChung commented 9 years ago

Yes, it's fixed now. The computation is a bit of a bodge - take a look at the PR or the master branch on my fork to see it, and it disagrees with KER slightly.

ChrisViral commented 9 years ago

@BenChung The only real solution that I can come up with is to either put the SRB bell data into a singleton a la RealChute, or to jam it into a private static variable within the class. partPrefab is just returning the (literal) self object, and I can't seem to find any other way of getting at the original data.

That's probably what you're going to have to do. I tried a lot of things. Pulling it from the prefab doesn't work, as the confignode or objects are not serializable and lost. Reloading the part config using AvailablePart.configFullName works, but completely breaks down if ModuleManager is used in between. Serializing the data using this completely breaks the PartLoader (I know, I tried). The only thing that worked to make sure the data would persist was to stick it it in a static variable inside a a singleton. https://github.com/StupidChris/RealChute/blob/master/RealChute/PersistentManager.cs#L27-L28 Hope this helps, confignode serialization was pretty important and I'm fairly bummed it got nuked.

BenChung commented 9 years ago

@StupidChris Thank you for the help - I tried about half of what you did, then gave up and put it in the static variable. I wonder why they got rid of the serialization for confignodes - it is extremely annoying.

@ItMustBeACamel I'm having problems fixing this bug. If you attach a (vertically) scaled fuel tank to the bottom of a procedural decoupler, you end up with the fuel tank jumping up a ways through the decoupler every time the scene is reloaded. Do you have any suggestions as to what is causing this?

It's most certainly enabled by my keeping track of attachments through save/reload, but I think that the cause is buried somewhere within the code that reinitializes the shape of the fuel tanks. I just don't know enough about this part of the code to really debug it, however.

ChrisViral commented 9 years ago

@BenChung because Unity fucked up. They started throwing out warnings when there is a serialization loop within a class. The thing is Unity serializes a depth of 7 levels, but the warning happened in every single object on every single level. This results in a massive spam of the log whenever serializing a ConfigNode. That's why it was removed. Basically they would have had to either force Unity to not serialize the ConfigNodeList within each ConfigNode, which would inherently break ConfigNodes because you lose all nested data, or they would have had to reformat all of ConfigNodeList to patch it. They went with the "remove serialization and let the modders figure it out" solution.

ItMustBeACamel commented 9 years ago

@BenChung Can give me a short explanation on your changes, I haven't had much time for programming recently and I'm not sure whats wrong here yet. Seems like the "part attached message" gets received before the shapes are initialized right?

BenChung commented 9 years ago

@ItMustBeACamel The major new addition is a fix to the problem where parts would forget about their parents and children, a problem that I fixed in 7394a0. This showed up a whole host of new issues.

The first issue was that nodeOffset wasn't applied to attached nodes. This was fixed by making a new type of transformable that looked up the offset, and was the original bug I ran into. This caused parts attached to the bottom of SRBs to be inside the SRB bell after reloading, but this is no longer an issue.

The second issue, the one that I'm dealing with now, is that for some reason changes to the position of nodes aren't propagated through to the parts attached to those nodes. To see what's happening, attach two (procedural) SRBs radially onto a core, then put procedural nosecones on top of them. Save and reload, and you'll notice that the nosecones have sunken into the SRBs. The same effect happens with the decouplers too.

What is happening is that the parts are attached through nodes that start out invalid (as the shape hasn't been initialized yet), but then are changed to the correct position. However, the part that was attached to that node isn't moved to the new position. Instead, it is left in the old position, a position that puts it either above (procedural decoupler) or inside (procedural nosecone) the parent part.

To fix this, we need to update the attached part whenever the node's position changes, but this is proving difficult. Do you have any suggestions?

BenChung commented 9 years ago

To go into detail a bit more about the chain of events, here's what's happening:

  1. KSP loads the new procedural object, with attachment nodes in entirely the wrong place
  2. We initalize the shape, and call MoveAttachments
  3. MoveAttachments figures out the new position for the nodes
  4. MoveAttachments propagates this through to the nodes using the nodeFollower
  5. The nodeFollower adjusts the node (and only the node) into the correct position.

The issue is that the parts that were attached to that node stay at the old and really wrong original position, not the updated position.

Edit to add: this might be wholly wrong, however. Put most simply, the parts are acting as if lines 236 and 248 of ProceduralAbstractSoRShape don't exist, but the attach nodes work properly.

ItMustBeACamel commented 9 years ago

It might have something to do with invoking PartChildAttached in OnStart. The shapes doesn't actually get built until their first update cycle.

[edit] maybe not. seems like UpdateShape gets called in InitializeNodes()

Also: Since 0.90, part clipping is an allowed thing and it seems to be here to stay. So thats also a thing we must take into account. We cannot rely on the old "find the outside of the part and attach there" approach anymore. Maybe it's necessary to overhaul the whole way attachments work in PP.

BenChung commented 9 years ago

@ItMustBeACamel There's something even weirder going on. What's happening is that the node position is getting set correctly before attachment, but when we go to attach the tank it's not in the same place. By the way, the simplest reproduction case is two procedural tanks stacked on top of each other (and grown 1m). When you save and reload, they should be inside each other.

BenChung commented 9 years ago

Figured out the problem. We're doing it all in one go for node positioning and attachments, so only some procedural components have been updated when we go to attach them together. Hopefully, I'll have a fix soon...

ItMustBeACamel commented 9 years ago

yep I just figured it out too :D looks like not all parts get created in the same frame

BenChung commented 9 years ago

I have a fix, but it's quite ugly. How did you do it?

BenChung commented 9 years ago

Oh, there's another problem now. Struts don't work going from one PP part to another - you have to terminate on a non-PP component for some reason. Any idea why?

ItMustBeACamel commented 9 years ago

Actually they do get created on the same frame but Update gets called immediately after creation. Confusing stuff...

See here: https://github.com/ItMustBeACamel/ProceduralParts/commit/9f63d8a79c1900bf1a13f844aa43475b5d838364

Its what you already did with the message queue (but I took out the other parts to test it seperately). The difference is that I spool the queue on LateUpdate(). Seems to work.

on the struts: I have no idea. :D

BenChung commented 9 years ago

@NathanKell Do you have any idea what is going wrong with the struts? I'm at a dead end now.

ctbram commented 9 years ago

What is the status of an official 1.0 proc parts release. I really miss this mod as I use it to remove all the redundant fuel tank parts and save quite a bit of memory

On Sat, May 2, 2015 at 10:48 AM, Benjamin Chung notifications@github.com wrote:

@NathanKell https://github.com/NathanKell Do you have any idea what is going wrong with the struts? I'm at a dead end now.

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-98367848 .

BenChung commented 9 years ago

I would rather not do an official release until the strut problem is solved, as they're such an important part of the game. Otherwise, the unofficial release seems to be working generally pretty well.

ctbram commented 9 years ago

cool thx I have been half following the strut issue. Makes sense to hold off until it's resolved.

On Sat, May 2, 2015 at 12:16 PM, Benjamin Chung notifications@github.com wrote:

I would rather not do an official release until the strut problem is solved, as they're such an important part of the game. Otherwise, the unofficial release seems to be working generally pretty well.

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-98374464 .

ItMustBeACamel commented 9 years ago

I'm not sure what changed....but the struts problem seems to be gone. Also KSP throws no NREs anymore when trying to revert a flight after rerendering a drag cube. Maybe squad patched that out in 1.0.2

BenChung commented 9 years ago

Weird - I'm still seeing the struts problem, but it might be an artifact of the build. In that case, I think it's ready then.

On Tue, May 5, 2015 at 4:02 PM, ItMustBeACamel notifications@github.com wrote:

I'm not sure what changed....but the struts problem seems to be gone. Also KSP throws no NREs anymore when trying to revert a flight after rerendering a drag cube. Maybe squad patched that out in 1.0.2

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-99204315 .

BenChung commented 9 years ago

Wait, try connecting from a procedural part to another. Do you pick up the target part? That's what happens to me.

On Tue, May 5, 2015 at 4:35 PM, Benjamin Chung bwchung@andrew.cmu.edu wrote:

Weird - I'm still seeing the struts problem, but it might be an artifact of the build. In that case, I think it's ready then.

On Tue, May 5, 2015 at 4:02 PM, ItMustBeACamel notifications@github.com wrote:

I'm not sure what changed....but the struts problem seems to be gone. Also KSP throws no NREs anymore when trying to revert a flight after rerendering a drag cube. Maybe squad patched that out in 1.0.2

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-99204315 .

ItMustBeACamel commented 9 years ago

Works for me. although there are floating strut connectors when parts get resized. Have you pulled the commits I pushed earlier?

BenChung commented 9 years ago

No - I'll try them.

On Tue, May 5, 2015 at 4:54 PM, ItMustBeACamel notifications@github.com wrote:

Works for me. although there are floating strut connectors when parts get resized. Have you pulled the commits I pushed earlier?

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-99219067 .

BenChung commented 9 years ago

Yes, it works. Weird.

ItMustBeACamel commented 9 years ago

Alright release is ready. @OtherBarry can you please update the OP?

NathanKell commented 9 years ago

Shoot, I was supposed to get TestFlight integration in before release. Oops.

ItMustBeACamel commented 9 years ago

Wanted to get that out to see if everything works so far. There are still some urgent issues so I think there should be another update soon anyway.

NathanKell commented 9 years ago

Gotcha, makes sense. I'll try to detach from Kopernicus long enough to do that then. :)

OtherBarry commented 9 years ago

OP updated. Sorry for delays...

On Fri, May 8, 2015 at 12:23 PM, ItMustBeACamel notifications@github.com wrote:

Alright release is ready. @OtherBarry https://github.com/OtherBarry can you please update the OP?

— Reply to this email directly or view it on GitHub https://github.com/Swamp-Ig/ProceduralParts/issues/121#issuecomment-100076846 .