Skip to content

Conversation

@goodroach
Copy link
Contributor

This is the basic framework behind the node directors. Features include:

  • Fully functional multi-crew system with directors where each director can be assigned to multiple turrets in sets.
  • Works for AA as well
    Problems:
  • AA is very laggy at the moment because it scans through the ship to find the signs each time. This will be remedied if we can keep track of all the subcraft signs in a craft.

@goodroach goodroach changed the title Dev goodroach nodedirector Add nodal directors Aug 6, 2023
@drfiveminusmint
Copy link
Contributor

I'm going to be honest, I don't like this change. At least as Goodroach described it to me, it will break a lot of currently used ships, as you have to keep the director signs blank if they're not being used. Not worth it IMO.

@DaarkKarma
Copy link

Personally as a player and someone looking to make a server, I think it's a great idea. Players can always change their ships, which if it's just changing the signs is easy enough. Or if it's that much of a problem, a config option that toggles this ability for the server?

@drfiveminusmint
Copy link
Contributor

This is a recurring issue I see in Movecraft's development cycle: Someone pushes a breaking change because they thought it'd be cool, doesn't think about the implications, and then we have to spend a bunch of development time reverting/disabling the feature. Just look at the issues with gears on release for an example.

@DaarkKarma
Copy link

The issue you originally explained, and the gears issue are two different kinds of issues. The issue you're worried about for this contribution is involving pre-existing ships. Implying that those ships can just be changed. Which isn't exactly an uncommon thing when it comes to plugin updates. Unless you're implying it will break the craft files themselves then that would be a problem. But as you mentioned the signs specifically, I get the impression that just removing the sign then placing it again with the new version would fix the problem for players and the ships they already built. Inconvenient but hardly a persistant issue.

In any case, why not test it? put a version with this change into a preexisting test server and confirm how big or small an issue from this would be?

@goodroach
Copy link
Contributor Author

This is a recurring issue I see in Movecraft's development cycle: Someone pushes a breaking change because they thought it'd be cool, doesn't think about the implications, and then we have to spend a bunch of development time reverting/disabling the feature. Just look at the issues with gears on release for an example.

So all you have to do is to change the usage of cannon director signs, the rest are of no consequence. What I could also do is to just default to the normal cannon director if no sign strings on the craft are found.

@Silent-Knight1
Copy link

This is a recurring issue I see in Movecraft's development cycle: Someone pushes a breaking change because they thought it'd be cool, doesn't think about the implications, and then we have to spend a bunch of development time reverting/disabling the feature. Just look at the issues with gears on release for an example.

This isn’t the fault of the person who created the idea; seeing what changes are being made and thinking about the immediate effects are ultimately the responsibility of the development team. You know what is right and how the game should run, and are therefore the filter these changes must pass through.

Secondly, this feature could actually push forward crewed ships, unlike current Directors which make commanding a vessel with a single pilot as easy as possible. This is simply another layer which creates an option for players, which is indicative of a ‘good’ addition.

@goodroach
Copy link
Contributor Author

goodroach commented Oct 8, 2023

Update:

  • Additional optimizations for checking the sign locations by only checking for sign locations once, and only checking if the craft has moved.
  • Nodal directors are optional, and additional strings that do not correlate to any turret will allow directions to function as before.
  • Refactored a few methods to reduce duplicate code.

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.

4 participants