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

folding bicycle is dead. Long live Folding Bicycle #46805

Closed
wants to merge 5 commits into from

Conversation

hexagonrecursion
Copy link
Contributor

Summary

SUMMARY: Bugfixes "Reimplement folding bicycle as a regular foldable vehicle that follows the same rules as the rest"

Purpose of change

Fixes #46752
Fixes #28976
Fixes #39399

Folding bicycle was a flodable vehicle before flodable vehicles were implemented. Now we have a more general foldable vehicle system, but we are still keeping folding bicycle as a special case which follows different rules. It is time to reimplement folding bicycle as a regular foldable vehicle.

Describe the solution

  • reimplement or remove content that references "folding_bicycle"
  • add migration code to replace the folding_bicycle item in save files with the new version
  • add migration code to replace the folding bicycle vehicle in save files with the new version

Describe alternatives you've considered

Testing

none yet

Additional context

none

@BrettDong BrettDong added <Bugfix> This is a fix for a bug (or closes open issue) Vehicles Vehicles, parts, mechanics & interactions labels Jan 16, 2021
As far as I know there is unfortunately no way to reference a folded version of a vehicle from inside an item group and implementing one would probably be more effort than it is worth.
@hexagonrecursion
Copy link
Contributor Author

hexagonrecursion commented Jan 16, 2021

As far as I know there is unfortunately no way to reference a folded version of a vehicle from inside an item group and implementing one would probably be more effort than it is worth. So far I simply deleted the entry from item groups. Not sure what to replace this content with.

@hexagonrecursion
Copy link
Contributor Author

All that is left it on the C++ side:

  • cleanup dead code
  • implement save migration (since jsonized migration is probably not flexible enough)

@hexagonrecursion
Copy link
Contributor Author

Yes. Definitely not flexible enough

class migration
{
public:
itype_id id;
itype_id replace;
std::set<std::string> flags;
int charges = 0;
class content
{
public:
itype_id id;
int count = 0;
bool operator==( const content & ) const;
void deserialize( JsonIn &jsin );
};
std::vector<content> contents;
bool sealed = true;
};

@int-ua
Copy link
Contributor

int-ua commented Jan 17, 2021

I think I mislabeled the issue as a Good first issue. It's far from trivial.

@Rivet-the-Zombie
Copy link
Member

As far as I know there is unfortunately no way to reference a folded version of a vehicle from inside an item group and implementing one would probably be more effort than it is worth. So far I simply deleted the entry from item groups. Not sure what to replace this content with.

So this PR prevents folding bicycles from spawning outside of a single profession's starting inventory?

@hexagonrecursion
Copy link
Contributor Author

So this PR prevents folding bicycles from spawning outside of a single profession's starting inventory?

Good point. I should make the unfolded version spawn randomly somewhere.

@hexagonrecursion
Copy link
Contributor Author

I think I mislabeled the issue as a Good first issue. It's far from trivial.

The hardest part is meeting my archenemy - c++ compilers and not running away screaming like a little girl.

@hexagonrecursion
Copy link
Contributor Author

  • add migration code to replace the folding_bicycle item in save files with the new version
  • add migration code to replace the folding bicycle vehicle in save files with the new version

I came up with a simpler solution:

  • remove the ability to fold the folding bicycle vehicle
  • make the folding_bicycle item disassemble into parts that you can build your own folding bicycle

This has the downside of breaking existing folding bicycles, but the upside is less code to maintain.

@ZhilkinSerg
Copy link
Contributor

Let's move it post-0.F

@Night-Pryanik
Copy link
Contributor

@hexagonrecursion is this still being worked on?

@hexagonrecursion
Copy link
Contributor Author

No. Sorry. Anyone, fell free to pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
6 participants