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

WIP: feat: add active effects to items #762

Closed
wants to merge 31 commits into from

Conversation

jonepatr
Copy link
Collaborator

@jonepatr jonepatr commented Dec 29, 2021

NOT READY FOR MERGE as it depends on the migration PR.

  • Please check if the PR fulfills these requirements
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    feature

  • What is the current behavior? (You can also link to an open issue here)
    Previsouly adding a trait did not affect the actor.

  • What is the new behavior (if this is a feature change)?
    This change makes
    it so that a trait is an active effect and can change the
    actor.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    It comes with a migration to ensure backward compability.

  • Other information:
    Fix 272

@jonepatr jonepatr force-pushed the use-active-effects-for-traits branch 3 times, most recently from 2cb9f69 to 745c3ae Compare December 29, 2021 15:55
@jonepatr jonepatr changed the title NOT READY FOR MERGE: WIP: feature: traits are now active effects NOT READY FOR MERGE: WIP: feat: traits are now active effects Dec 30, 2021
@jonepatr jonepatr linked an issue Jan 1, 2022 that may be closed by this pull request
@mergify
Copy link

mergify bot commented Jan 2, 2022

This pull request is now in conflicts. Could you fix it? 🙏

@jonepatr jonepatr force-pushed the use-active-effects-for-traits branch from 745c3ae to ac532e6 Compare January 2, 2022 19:55
@mergify
Copy link

mergify bot commented Jan 2, 2022

This pull request is now in conflicts. Could you fix it? 🙏

@jonepatr jonepatr force-pushed the use-active-effects-for-traits branch from ac532e6 to 3d15245 Compare January 3, 2022 04:52
@mergify
Copy link

mergify bot commented Jan 3, 2022

This pull request is now in conflicts. Could you fix it? 🙏

@jonepatr jonepatr force-pushed the use-active-effects-for-traits branch from 990dca8 to c735dc4 Compare January 3, 2022 20:27
@jonepatr jonepatr changed the title NOT READY FOR MERGE: WIP: feat: traits are now active effects WIP: feat: traits are now active effects Jan 3, 2022
@jonepatr jonepatr force-pushed the use-active-effects-for-traits branch from a180667 to 866c543 Compare January 4, 2022 02:17
@marvin9257
Copy link
Collaborator

marvin9257 commented Jan 4, 2022

Oddly, I'm getting this error when the system initializes. Not certain why. Let me check in the base system.

Screen Shot 2022-01-04 at 7 38 53 AM

Just checked with version 1.3.6 - no error

@marvin9257
Copy link
Collaborator

marvin9257 commented Jan 4, 2022

What is this supposed to look like? I'm not certain what the header ("starting with "Attribute Key") does.
Screen Shot 2022-01-04 at 7 56 40 AM

At least, there is a localization missing.

@marvin9257
Copy link
Collaborator

When I edit a new effect - I get this warning.
Screen Shot 2022-01-04 at 8 21 30 AM

I think it has to do with the color not having an initial value. Is there a way to initialize as black or white when called?

@jonepatr
Copy link
Collaborator Author

jonepatr commented Jan 4, 2022

Oddly, I'm getting this error when the system initializes. Not certain why. Let me check in the base system.

Screen Shot 2022-01-04 at 7 38 53 AM

Just checked with version 1.3.6 - no error

This has to do with the new dynamic import stuff I'm pretty sure. From the looks of it, I think you have a .DS_Store file in the hooks folder. The dynamic import takes all files in the hooks/template/migrations folder and adds them to the system. For now, try removing the .DS_Store. The proper sollution is to filter out those files in the rollup.config.js script

@jonepatr
Copy link
Collaborator Author

jonepatr commented Jan 4, 2022

What is this supposed to look like? I'm not certain what the header ("starting with "Attribute Key") does. Screen Shot 2022-01-04 at 7 56 40 AM

At least, there is a localization missing.

So the three headers there were supposed to be the same as if you go into the active effect and add effects.

@jonepatr
Copy link
Collaborator Author

jonepatr commented Jan 4, 2022

When I edit a new effect - I get this warning. Screen Shot 2022-01-04 at 8 21 30 AM

I think it has to do with the color not having an initial value. Is there a way to initialize as black or white when called?

Hm, what is it that should have a color? the item or the active effect? or something else?

@marvin9257
Copy link
Collaborator

marvin9257 commented Jan 4, 2022

Yes, it is when I put the edit effect button.
When the window opens the tint color is blank. If it is set to something the warning message doesn't appear.
Screen Shot 2022-01-04 at 9 26 48 AM

@marvin9257
Copy link
Collaborator

.DS_Store

Hmm...I can't find this file in the folder - or anywhere when I search - except for another program

@jonepatr jonepatr force-pushed the use-active-effects-for-traits branch from 6892f37 to cca7da2 Compare January 21, 2022 23:43
@marvin9257
Copy link
Collaborator

I went through the Active effects PR and it looks great. I pushed a commit with some suggested styling tweaks. I also fixed a missing label for the effect on create. One thing I noticed it that if the item is dropped into a ship's locker, the active effect can't be edited. Is that the desired function? (duplicate message from discord)

@mergify
Copy link

mergify bot commented Jan 27, 2022

This pull request is now in conflicts. Could you fix it? 🙏

1 similar comment
@mergify
Copy link

mergify bot commented Sep 24, 2022

This pull request is now in conflicts. Could you fix it? 🙏

@marvin9257
Copy link
Collaborator

Replaced by #1197

@marvin9257 marvin9257 closed this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants