Demigiant / dotween

A Unity C# animation engine. HOTween v2
http://dotween.demigiant.com
Other
2.32k stars 348 forks source link

Package Manager and assembly definitions #251

Open metamorphling opened 5 years ago

metamorphling commented 5 years ago

Are there any plans for moving onto unity's new package management system and usage of assembly definitions? Nothing urgent here, just an enhancement idea. Tried to move "Demigiant" into "Packages", added packages.json and registered it with manifest - so far works well. Utility Panel is dead though, probably it wants dotween folder to be at hardcoded place? I'm sure you are really busy, but just for the record dropping a few links on what I am talking about

https://docs.unity3d.com/Packages/com.unity.package-manager-ui@1.7/manual/index.html https://docs.unity3d.com/Manual/ScriptCompilationAssemblyDefinitionFiles.html

Besides utility panel, as to what I had to do to make assembly definitions work with DOTween: 1) Create core asmdef file in "Modules" folder Seeing how project went modular way, there is a potential to breakdown every module into it's own folder and give asmdef file for every folder. That way switching modules will be as easy as having a core asmdef which adds/removes references to module assembly definitions. 2) Put scripts from "DOTweenPro" into "DOTweenPro/Scripts" and create pro asmdef file in Scripts For some reason if I created asmdef in DOTweenPro as is some strange errors were popping up, got the feeling asmdef was fighting dll. 3) Reference both core and pro asmdefs from source code

favoyang commented 3 years ago

One potential blocker is that @yoshida190 has taken your namespace/package ID com.demigiant.dotween, despite not owning demigiant.com. Hoping they will respond here as they have issues disabled on their fork.

Regarding the taken package ID - I'm pretty sure you can just ask @favoyang (the creator of OpenUPM) to transfer the ownership if needed.

I have received a request from @yoshida190 to retire his fork from OpenUPM, as long as anyone like to provide a replacement, either the official one or a maintainable fork. Please create an issue on OpenUPM to propose your alternatives.

rhys-vdw commented 3 years ago

@rhys-vdw Even if it would be available on OpenUPM, I don't think that'll work with the pro version since then, there would be no reason to actually buy it. I'm talking about the latest version available in the asset store through "My assets" in the regular package manager.

I don't know anything about the pro version. I assume it's not open for pull requests. But generally speaking there's no reason why the exact file structure of a well structured unity package can't be directly dropped into a unity project, so whatever changes are made can be applied to both.

Fwiw I am using asmdefs with latest version on GitHub and it's working perfectly.

rhys-vdw commented 3 years ago

I have received a request from @yoshida190 to retire his fork from OpenUPM, as long as anyone like to provide a replacement, either the official one or a maintainable fork.

Thanks @favoyang.

I think maintaining a fork would not be ideal here as ultimately DOTween itself should be updated to support modern development practises.

Also I suspect it will be hard to automate the updating process, and setting up CI is a bit more than I'm signing up for here.

I'll wait for @Demigiant to consider my proposal.

lorinatzberger commented 3 years ago

So, any news on this? I understand that it may be a more complicated situation but it's been 2 whole years. As it is, I can't even run the setup since it throws errors right after importing.

lorinatzberger commented 3 years ago

Yeah, giving up on making this run again. I tried deleting anything about the pro version to simplify things then making 4 asmdefs for DemiEditor, DemiLib, DoTweenEditor(including both DoTweenEditor and DoTweenUpgradeManager) and DoTween. I set the correct editor settings and used Override References to link the asmdefs to their respective dlls.

All I get is the console spamming things constantly: TypeLoadException: Could not load type of field 'DG.DOTweenEditor.UI.DOTweenUtilityWindowModules:_src' (14) due to: Could not resolve type with token 01000012 (from typeref, class/assembly DG.Tweening.Core.DOTweenSettings, DOTween, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null) assembly:DOTween, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null type:DG.Tweening.Core.DOTweenSettings member:(null) signature:<none> System.RuntimeType.GetMethodsByName (System.String name, System.Reflection.BindingFlags bindingAttr, System.Boolean ignoreCase, System.RuntimeType reflectedType) (at <695d1cc93cca45069c528c15c9fdd749>:0) System.RuntimeType.GetMethodCandidates (System.String name, System.Reflection.BindingFlags bindingAttr, System.Reflection.CallingConventions callConv, System.Type[] types, System.Boolean allowPrefixLookup) (at <695d1cc93cca45069c528c15c9fdd749>:0) System.RuntimeType.GetMethodImpl (System.String name, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, System.Reflection.CallingConventions callConv, System.Type[] types, System.Reflection.ParameterModifier[] modifiers) (at <695d1cc93cca45069c528c15c9fdd749>:0) System.Type.GetMethod (System.String name, System.Reflection.BindingFlags bindingAttr) (at <695d1cc93cca45069c528c15c9fdd749>:0) DG.DOTweenUpgradeManager.Autorun.ApplyModulesAndASMDEFSettings () (at D:/DG/_Develop/__UNITY3_CLASSES/_Holoville/__DOTween/_DOTween.Assembly/DOTweenUpgradeManager/Autorun.cs:44) DG.DOTweenUpgradeManager.Autorun.OnUpdate () (at D:/DG/_Develop/__UNITY3_CLASSES/_Holoville/__DOTween/_DOTween.Assembly/DOTweenUpgradeManager/Autorun.cs:30) UnityEditor.EditorApplication.Internal_CallUpdateFunctions () (at <01e901910c5242498b7e69f6d2d8d500>:0)

I also did somehow managed to get the setup window to show at some point but clicking on the setup button just threw an error.

It's not even about the fact that it should necessarily work out of the box but a guide would be really nice.

krisrok commented 2 years ago

@Demigiant daniele, help us out here :)

Maestroxr commented 2 years ago

crickets

rhys-vdw commented 2 years ago

Still willing to do this one if approved.

Alternatively I could look into making a fork that uses github actions to pull in upstream changes? I'd need a bit of time to research but it could be the best stopgap while we wait for official support.

Demigiant commented 2 years ago

Hey! Sorry for the radio silence here but every time I see a UPM notification my mind explodes, then I make plans to try doing it again, then either try and fail or cancel the idea; and lately I let these messages go over my head (so much that I missed the fact that the com.demigiant.dotween namespace had been taken, which is not great). I tried to implement it many times in the past, and in various ways, but always had show-stopping problems: both because of the modules system that needs to re-write files and because anyway that would never work with the Pro. I humbly ask you to please continue using the UPM-AssetStore side of it, or if you really need to include stuff for clients/projects you can simply include DOTween's files in the project (that is perfectly allowed, as long as it's not the Pro)? I know it's nicer to have it working directly from UPM, but it's not that bad to have to go through the Asset Store, or am I missing something obvious? Honest question, agh 👀💦

rhys-vdw commented 2 years ago

Thank you for replying @Demigiant!

sorry for long message but I want to make my case

I really love this library and it bugs me that it's not using up-to-date practises.

because of the modules system that needs to re-write files

Actually there is support for this in UPM. (I explained above.) Basically Unity will set compile flags based on presence of other dependencies, so this whole custom module system could be removed and replaced with the native support. Your module system, while clever, is made redundant by this Unity feature. (actually this is one of the things I personally would most like to remove)

anyway that would never work with the Pro

Actually it's completely fine. Customers can put your paid package right into the Packages/ folder in the project root. Unity will treat it as local UPM package. I've seen other paid assets that are distributed this way.

(I believe you can also just drop a valid UPM package into Assets/ and it will work too, but it's probably fine to drop support for pre-UPM versions of Unity at this stage.)

I missed the fact that the com.demigiant.dotween namespace had been taken, which is not great

See above where OpenUPM maintainer confirms that you can take back the namespace if an official release is made.

I know it's nicer to have it working directly from UPM, but it's not that bad to have to go through the Asset Store, or am I missing something obvious?

From my perspective there are a bunch of things that could be nicer:

  1. Upgrading to latest release (or maintaining a fork) is complicated because you can't just change the reference in package manager.
  2. It's hard/impossible to work out which version of DOTween you have installed in a project. (Whereas the UPM dependency would just tell you in the packages.json.)
  3. Currently it's not possible to distribute optional extensions for DOTween via UPM. UniTask has a suite of DOTween extensions. It uses the Unity dependency system to enable its extensions based on the presence of the com.demigiant.dotween package. These extensions cannot be used without relying on the old, bad, broken fork of DOTween, or manually copying the UniTask extensions into your assets.
  4. Distributing straight from git/OpenUPM makes source easily accessible.
  5. Upgrading/downgrading a UPM package does not have the problem of old renamed or deleted files remaining, it always does a full immutable copy of the version you want.
  6. As a consumer it's preferable for dependencies to not be included under version control. This prevents accidental modification of dependencies and repo bloat.
  7. The current implementation for some reason deletes any asmdef that is added to the plugins folder, which means that you cannot use your modules at all if you're using asmdefs, as there's no way to reference them from your assembly! (this is a separate issue but damn it's annoying)

I tried to implement it many times in the past, and in various ways

As I mentioned above I've done a few of these now and have researched how to make the DOTween module system work. I'm confident that I can do it. Obviously you'd need to merge the changes to the pro version as I have no repo access.

RunninglVlan commented 2 years ago

I like UPM packages too, but what if the package user wants to modify it, e.g., to make a fix or extend its functionality? Using Asset Store packages we could just make a modification, but how to do it using UPM? Do we need to copy it first to /Packages? Or make a fork and modify it instead?

rhys-vdw commented 2 years ago

Do we need to copy it first to /Packages? Or make a fork and modify it instead?

These are the two options, yes.

Or if you don't share my concerns you can just download the asset store version. This would be for people who want the UPM version. The manifest is just ignored if you grab the package from the store (or the dotween website which is where I get it from)

Just to be completely clear, this would add the ability to distribute via UPM, not make DOTween incompatible with store or private distribution (for the pro version). That would be unacceptable as I'm sure the majority of users (esp. beginners) will still be getting it from the asset store.

mikerochip commented 2 years ago

@Demigiant If you're finding yourself trying to do the work and giving up, then just let @rhys-vdw do it. That's the beauty of being open source on git and hosting on GitHub. The problem is that nobody besides yourself has repo access. Here's how you can do it:

  1. Create a branch called upm and push it
  2. Let @rhys-vdw make the code UPM compatible, then he can open a Pull Request against the upm branch
  3. You accept and merge the Pull Request

That's it. No heavy lifting for you and everyone that wants UPM support benefits.

With this method, you can leave all your asset store stuff as it is. This GitHub repo would just provide additional upm support via the branch.

Demigiant commented 2 years ago

Since it's the holidays, let's keep this as a conversation for now, so I can understand better and make up my mind :)

@rhys-vdw I understand your point of view but I have a few questions/doubts:

Actually there is support for this in UPM. (I explained above.) Basically Unity will set compile flags based on presence of other dependencies, so this whole custom module system could be removed and replaced with the native support. Your module system, while clever, is made redundant by this Unity feature. (actually this is one of the things I personally would most like to remove)

The problem is that, yes, I know that, but that would work only in recent versions of Unity, while I always strived to keep DOTween as retro-compatible as possible, because there's lots of teams still working on astoundingly old versions of Unity (just a couple weeks ago I had an email from a team which had problems updating because the recent versions of DOTween require Unity 5.6 while they were still on 5.4). So the self-rewriting modules are imo the best way (other than having DOTween add custom defines, like it did before, but which is a practice I really didn't like because it was too invasive). This is also why I see the UPM approach as limited: I don't think Unity gave it the love it deserved, and it's only for recent versions.

It's hard/impossible to work out which version of DOTween you have installed in a project. (Whereas the UPM dependency would just tell you in the packages.json.)

Can you explain me better here? Afaik UPM doesn't add custom defines, but maybe I'm missing something. Right now you can determine DOTween's version visually by opening DOTween's Utility Panel, or code-wise by querying DOTween.Version

Upgrading/downgrading a UPM package does not have the problem of old renamed or deleted files remaining, it always does a full immutable copy of the version you want.

This I love, because the leftover files is one of the main reason I made DLLs instead of loose scripts

The current implementation for some reason deletes any asmdef that is added to the plugins folder, which means that you cannot use your modules at all if you're using asmdefs, as there's no way to reference them from your assembly! (this is a separate issue but damn it's annoying)

Agh! Can you explain me more? I'm not sure I understand: ASMDEFs are working on my side

@mikerochip

If you're finding yourself trying to do the work and giving up, then just let @rhys-vdw do it. That's the beauty of being open source on git and hosting on GitHub. The problem is that nobody besides yourself has repo access.

Your words are wise indeed, but I always preferred to keep direct control of the repo because of many reasons. That said, I will ponder this seriously while we have this discussion.

Arcnor commented 2 years ago

If you're finding yourself trying to do the work and giving up, then just let @rhys-vdw do it. That's the beauty of being open source on git and hosting on GitHub. The problem is that nobody besides yourself has repo access.

Your words are wise indeed, but I always preferred to keep direct control of the repo because of many reasons. That said, I will ponder this seriously while we have this discussion.

Just my 2c as an outsider "lurker" interested in this issue for a long time: I don't think the posters above meant you giving or even losing control of the repo, but working on a pull request basis, which still needs your approval/merging before any changes happen, but those can start happening independent from you, which is the good thing, I think :).

rhys-vdw commented 2 years ago

Sorry yet again for another short essay! Don't feel there's an time pressure on responding

Since it's the holidays, let's keep this as a conversation for now, so I can understand better and make up my mind :)

Hey @Demigiant. Yeah for sure. I think it's a complicated problem to solve and were you to accept my help I'd need to ask you questions along the way, as well as make a proposal for your approval before I start. My main concern is doing a bunch of work and it not being appropriate. I wouldn't want to waste your or my time, nor put you in a position where you felt obligated to accept work that you do not feel comfortable with.

Also I originally made the offer a while back where I had a chunk of time available, but I will find time to do it still.

...I always strived to keep DOTween as retro-compatible as possible...

Definitely a problem to grapple with, I'll keep that in mind. Might need a bit more thinking. Older versions of Unity will simply ignore asmdefs, which should afford some freedom.

If you hadn't mentioned that you objected to adding defines then I might suggest telling consumers to add them manually or create a simple wizard that does it. It is a bit annoying (I've had dependencies that do it). I'd say it's a small price to pay for running a long unsupported version of Unity and still getting updates, but I'm sure there's a way to keep the current module system for .unitypackage releases if that is determined to be the best option.

I don't know the details of how you support older Unity versions, or how your build + deploy pipeline works. Probably worth mentioning that I'd need to have a proper read of the codebase before I can lay out what compromises must be made (if any).

Can you explain me better here? Afaik UPM doesn't add custom defines, but maybe I'm missing something. Right now you can determine DOTween's version visually by opening DOTween's Utility Panel, or code-wise by querying DOTween.Version

Ah cool. I had never considered that. I have just looked in the README in the past to find a version because that's the place I thought to look, but the version does not seem to be in there. So instead I am just vigilant with including the installed version in git so I can search back through my history for it.

I guess just as a comment it's easier to have a standard way of checking for updates (open up package manager and it tells you the version and whether an update is available, click once and you're up to date) rather than looking for each dependency's version and cross-referencing it with the website/asset store, deleting the existing folder, downloading the new package, extracting it etc.

the leftover files is one of the main reason I made DLLs instead of loose scripts

Yeah, I had a suspicion that that was why. It's an annoying flaw of the old system.

Can you explain me more?

Okay, so I just ran into this two weeks ago integrating the package with a new project.

In my case I have Assets/_Game/Game.Runtime.asmdef which contains my projects code (I think this is pretty standard). This asmdef can only see code in DLLs or asmdefs that are explicitly referenced by it. There is no asmdef in Assets/DOTween/Modules, presumably because it assumes the old non-asmdef system where all code is grouped into the main assembly. This means that my assembly can't use the modules (regardless of module system changes). So typically the solution is you add an asmdef to the folder so that you can include it in your project.

However when I add in an asmdef it's immediately deleted with this confusing error:

image

As a result I needed to copy out the modules and put them somewhere else where your library doesn't do this. It's very confusing. I would argue that this kind of automatic self-modification is as invasive as a helper that modifies your build defines, but perhaps I don't have the full picture here.

(In any case the obvious fix here is to add the asmdef in the distributed package, I just mention it as an example of non-standard and confusing behaviour)

I always preferred to keep direct control of the repo

I take this to mean that you mean that you do most of the bug fixes and features yourself? I understand.

I saw the popularity of this issue and thought "hey, it's a big ask of the maintainer to do this work, and I think I could do this". I like the idea of the challenge and contributing to a great library. I understand what it is to have a codebase you're proud of, and won't be offended if you don't take me up on it.

Alternatively I'd be happy to take a look at help formulate a plan for you to execute without me, or honestly to just continue using it in its current form and despite my (admittedly minor) packaging quibbles.

Hope that helps. Happy holidays!

rhys-vdw commented 2 years ago

As a followup I have discovered that if I name the asmdef by a different name it doesn't get deleted. I'm guessing you'll have a better understanding of why this happens. Quick search suggests it's coming from here.

krisrok commented 2 years ago

However when I add in an asmdef it's immediately deleted with this confusing error

Have you tried generating asmdefs via the DOTween setup dialog? Maybe there‘s a watcher only allowing asmdefs created that way for some reason?

(Great there‘s some motion in here now, yay!)

rhys-vdw commented 2 years ago

Have you tried generating asmdefs via the DOTween setup dialog? Maybe there‘s a watcher only allowing asmdefs created that way for some reason?

Yeah I'm realising now that that's what's going on. No I had no idea, I just saw the package didn't have an asmdef so I added it using the standard naming convention.

bdovaz commented 2 years ago

My advice is that if you want to have backward compatibility with versions that do not support *.asmdef files, those developers should be penalized and should be the ones who have to perform steps manually (being documented) and not those who remain in LTS versions of Unity which is the usual.

It doesn't make sense for a team to have an active development, no matter how complex it is in version 5.x today... For 5.4.x we are talking about the year 2016 and we are almost in 2022... Any active development that is not on a supported LTS version is a mistake.

I would solve it in the following way:

For different modules I would split it into different *.asmdef files. Example: DOTween.Ugui, DOTween.Physics, ...

Each of those *.asmdef will have a "version define":

https://docs.unity3d.com/2020.3/Documentation/Manual/ScriptCompilationAssemblyDefinitionFiles.html#define-symbols

Example:

Resource = com.unity.modules.physics. Define = DOTWEEN_USE_PHYSICS Version = 1.0.0

And if I remember correctly the actual module system you have created changed a "#if true / false" when editing the configuration window. It would be to change that "#if true" to "#if DOTWEEN_USE_PHYSICS".

This solution would work fine with Unity 5.x instances because you would simply have to manually create the defines in Player Settings for "DOTWEEN_USE_PHYSICS", etc.

If you need help, I'm willing to create the PR without any problem.

gromilQaaaa commented 2 years ago

Hello from 2022. Request is still "Open". Its a shame...

Romaleks360 commented 2 years ago

still waiting for this! go go demigiant we can do this!

rhys-vdw commented 2 years ago

still waiting for this! go go demigiant we can do this!

I think it's safe to assume, at this stage, that it's not going to happen. I've been toying with the idea of making an MIT licensed alternative, but I'm not sure when, if ever, I'll get a chance.

laicasaane commented 2 years ago

I've been toying with the idea of making an MIT licensed alternative

There are already various MIT licensed tweening libraries, should we consider contribute to one of them instead of starting a new one?

Demigiant commented 2 years ago

Sorry if I'm keeping this in such low priority, but all the ideas I or other people had are imo either an architectural step backward or don't fully work, and I have to admit that I don't see keeping it as a normal asset as a big problem. That said, I keep this open because sometimes I get back to it (even if, yes, very low priority at this point: package manager drives me crazy 👀)

krisrok commented 2 years ago

architectural step backward or don't fully work package manager drives me crazy 👀)

Maybe at this point you could already start to think about nuget compatibility instead… Unity teased a bit about it during GDC. So it‘ll take another 4-5 years but then you could be an early adopter ;D

bdovaz commented 2 years ago

and I have to admit that I don't see keeping it as a normal asset as a big problem.

@Demigiant is a big problem when you create a package and you want to have DOTween as a dependency...

Apart from all the advantages of being a package, to mention a few: being able to change version easily just by editing the manifest.json file, having all the assets outside the /Assets folder, the fact that it is a package forces you to use the *.asmdef files with the advantages that entails.

krisrok commented 2 years ago

@Demigiant is a big problem when you create a package and you want to have DOTween as a dependency...

While all this is true, we have to admit upm is in a shitty state. Using dotween as dep in another package does nothing in vanilla upm. You’d have to use openupm-cli or @mob-sakai‘s helper packages (do they even still work in 2021+?).

So unless Unity fixes upm (will not happen) for many (most?) users it‘d still be manual steps to get the dependencies — whether via upm, git, unitypackage or whatever. So even though I don‘t like the fact I think @Demigiant still has a point being on the fence about this.

bdovaz commented 2 years ago

Of course, being in UPM format by itself is useless if it is not uploaded in an openupm-like npm registry (in which there is already a fork of DOTween in package format for years).

rhys-vdw commented 2 years ago

Of course, being in UPM format by itself is useless if it is not uploaded in an openupm-like npm registry (in which there is already a fork of DOTween in package format for years). — @bdovaz

Unfortunately this fork is both broken/incomplete (at my last time of using it) and violates the license. I was asked to maintain the fork by @yoshida190 and @favoyang, but declined due to this restriction. Because of this license it's not possible to maintain a community fork. I'm trying to avoid reading the DOTween source so that, if I decide to make my own tweening library, I won't accidentally copy the implementation.

Also specifically you can rely on a UPM formatted package directly from GitHub, OpenUPM is just handy for resolving dependencies (as @krisrok mentions). Having DOTween in a upm package is handy for projects that consume it directly, the abililty to use it as a subdependency is a niche case IMO (although I agree it's a pain point for upm).

favoyang commented 2 years ago

Due to the license restriction, the com.demigiant.dotween package has been removed from OpenUPM (both from the website and the registry).

To avoid breaking your project, please switch to this git dependency in the packages/manifest.json. The license still allows a fork I think. And using the git dependency is not considered a redistribution.

Learn more about the license restriction:

Learn more about how to use manifest.json and the git dependency:

I respect Demigiant's choice for the license. I'm not pushing for a change like MIT.

Further submission using com.demigiant.dotween scope has been blocked, unless something changes.


Update: If using a public fork's git dependency is still considered a redistribution, you can fork the project, turn it into a private fork, then replace the UPM git dependencies with your private fork.

bdovaz commented 2 years ago

@Demigiant that is, without an effective replacement of the package in openupm, a package that has been providing a service that has not been officially provided for years has been removed from the repository, leaving thousands of developers without an alternative that does not require making modifications to the project. There are cases in which possibly are third party libraries that depend on that package of yoshida.

Wasn't it better for @Demigiant to give permission somehow to avoid having to remove it?

And finally, are you willing to accept a PR that turns it into a package as a first step to be officially uploaded later to openupm and as long as there are no breaking changes? I might be willing to do that but what I am not going to do is to work hours on it only to have it rejected out of hand. I am concerned because I see PRs like #343 that have gone unanswered for years being as simple as editing a txt.

Demigiant commented 2 years ago

Woah, lot to unpack (pun not intended 😬). I'm out today, but let me get back to you all tomorrow, so I can write with more attention.

bdovaz commented 2 years ago

@Demigiant don't forget us!

I have been analyzing the repository and I see that you have several c# projects that have hard dependencies on specific Unity editor version assemblies.

Tasks that I see as a minimum that need to be done:

I see two defines: COMPATIBLE and RIGIDBODY. What is the first one for?

And if you are worried about publishing in the asset store, there is this solution for packages: https://github.com/needle-tools/hybrid-packages

Demigiant commented 2 years ago

While all this is true, we have to admit upm is in a shitty state. Using dotween as dep in another package does nothing in vanilla upm. You’d have to use openupm-cli or @mob-sakai‘s helper packages (do they even still work in 2021+?).

@krisrok You are voicing a lot of my concerns indeed. Another problem that I have with package manager (which I probably mentioned time ago in this thread) is that it always felt very alpha to me, which is one of the reasons I just give up easily when it drives my crazy

Due to the license restriction, the com.demigiant.dotween package has been removed from OpenUPM (both from the website and the registry).

To be clear I never complained about the OpenUPM package breaking my license. I admit that initially I wasn't superhappy when I saw that there was one and I hadn't been asked about it, but I understood it and I was ok with it. @favoyang I would also (probably) be willing to change the license a little bit in order to allow the com.demigiant.dotween OpenUPM, if you're open to discuss it, but from what I understand it's something that is too complicated to maintain (as long as I keep DOTween's main repo like this), right?

I'm trying to avoid reading the DOTween source so that, if I decide to make my own tweening library, I won't accidentally copy the implementation.

I appreciate it @rhys-vdw, and best of luck if you go that way. When I made my first tween engine and then my second one (this one), I took the same approach with the tween engines I was using before (on Flash) so I could both avoid copying them by mistake, and create something that used its own philosophy. (Then one Unity tween engine that I won't mention, years ago, copied code from mine and I was a little annoyed)

@bdovaz There is legacy code but it's either completely disabled or in separate classes that are not compiled with anything. The COMPATIBLE define is on unused legacy code (which fixed a problem with Unity's structs on Win apps) and RIGIDBODY is part of legacy classes, from the time when Unity "extras" (like physics, UI, etc.) were enabled via defines instead than by auto-rewriting some loose scripts when importing DOTween (a big change I made a while ago because I really didn't like that I was forcing extra defines on users' projects).

The other changes are pretty big though. Making DOTween into loose scripts would break a lot of things, and I'm still not convinced: I'd really like to continue keeping the DLL approach I have now (yes, sorry, I'm still not convinced by the importance of UPM, also because I freelance to survive and I never worked on a project that was using it 👀—which doesn't mean I don't believe it's cool, quite the contrary, but it still puzzles me, probably because I associate it to the mess that is Unity's Package Manager)

Demigiant commented 2 years ago

P.S. To you all: I'm sorry if I continue to be so reluctant, and I hope I don't come out as dismissive. I appreciate all that you are saying (and those checkboxes looked great, @bdovaz, I love checkboxes)

bdovaz commented 2 years ago

@Demigiant If I create a PR that creates the package structure with dlls and coexisting with your current development, would you be willing to merge it? The idea would be to merge more or less what yoshida did but making everything work as it should.

favoyang commented 2 years ago

@Demigiant, thanks for the reply.

Demigiant has never pushed us to remove the published fork on OpenUPM. It's just safe for OpenUPM to remove the unlicensed package first, then discuss how to make it work like changing the license.

An OSI-approved license from spdx.org/licenses/ is encouraged (a dual license is okay).

However, you can keep the custom license as long as it conforms to

It would be good if you approve adding UPM support to this repository directly. Then OpenUPM will track and publish from this repository, which drives you happier I guess. Otherwise, a community-maintained up-to-date fork with UPM support will be used, like before.

favoyang commented 2 years ago

While all this is true, we have to admit upm is in a shitty state. Using dotween as dep in another package does nothing in vanilla upm. You’d have to use openupm-cli or @mob-sakai‘s helper packages (do they even still work in 2021+?).

@krisrok You are voicing a lot of my concerns indeed. Another problem that I have with package manager (which I probably mentioned time ago in this thread) is that it always felt very alpha to me, which is one of the reasons I just give up easily when it drives my crazy

It's Unity's decision to make the PackageManager (PacMan) strictly limit to the scope list, and DO NOT search packages from installed scoped registries. The scoped registry is designed to work with only a few scopes, pretty much just com.yourcompany.

Well, a public registry like OpenUPM is an accident for Unity. OpenUPM manages many scopes instead of just one. The limitation of PacMan is not friendly for us. It troubles every user. Eventually, I have to write the OpenUPM-CLI for crossing it.

Because of the Unity Package Guideline, it's not allowed to write a better packman GUI with a search scope behavior. So you have to use the CLI or follow the instruction from OpenUPM to update the scope list yourself (OpenUPM provides a list of scopes needed for a given package).

The barrier has been there for years. They know it, we know it.

bdovaz commented 2 years ago

@Demigiant @favoyang PR created: https://github.com/Demigiant/dotween/pull/586

I think I'm going to make a lot of people happy (myself included), or at least that's my intention.

Demigiant commented 2 years ago

Wait wait wait, @bdovaz, we were still in the (very slow, I admit) discussion phase! Like @favoyang suggested, other than modifying this repo, I would prefer to modify the license in order to allow UPM modifications and have it stored somewhere else. One of the reasons is that I really want to keep this main DOTween version compatible with older Unity versions (both because I know people that still use Unity 5, because I have a fixation on making stuff as retro-compatible as possible, and because there is basically no performance advantage in upgrading it to later Unity versions).

@bdovaz Since you already started working on it (it scared me that it was so sudden, but I also appreciate it), maybe you could manage the OpenUPM fork? That is, if we all agree and it sounds good. What do you think, @favoyang? Let me know: since we would make it official I'll also modify DOTween's credits to indicate who's managing that side.

If that all sounds good to you, I'll post a modified license here so you can let me know if it's ok (but after the weekend: I'm crazy busy because I'm closing the UI for a game and also having to deal with some hospital things—I'm ok, it's not that—plus I absolutely need a couple hours or more to fix a fringe DOTween bug that I thought I had fixed and that recently re-appeared—the indexOutOfRange exception, which is caught and dealt with but I don't want it to happen at all, obviously).

Let me know, and bear with my slowness a little more

Demigiant commented 2 years ago

P.S. just so you know: I always use the latest non-beta version of Unity when possible. I'm not one of the people using Unity 5, but still :D

favoyang commented 2 years ago

@bdovaz Since you already started working on it (it scared me that it was so sudden, but I also appreciate it), maybe you could manage the OpenUPM fork? That is, if we all agree and it sounds good. What do you think, @favoyang? Let me know: since we would make it official I'll also modify DOTween's credits to indicate who's managing that side.

It all sounds okay to me. This is just more work for @bdovaz to keep track of the upstream changes since he modified the directory structure. Git is not good at tracking files that moved, I assume it can not be automatically as a git pull upstream.

I'm also curious about the new license. But I'm positive that we can find something that works.

bdovaz commented 2 years ago

@Demigiant If the problem is 5.x compatibility I can multitarget the DOTween and DOTweenEditor projects to .NET Framework as before and to .NET Standard 2.0 for the package.

I "think" that is the only thing to modify to get it working again in versions prior to 2019.x (including 5.x).

My idea was to do this PR work so that it is in the official repository, not having to maintain a fork myself, the community wants to be maintained in an official way. Actually if we set it up well with CI it would be automatic and we wouldn't have to maintain much more since from what I see the development of DOTween (at least the free part) is quite stopped, I don't see much movement of commits.

This PR also greatly improves the flow of contributions because now we have with a workflow all the validation of the compilation of the package in multiple versions of Unity in an automated way.

I have no problem to help you to make the package flow as automatic as possible so that it has almost no maintenance but as long as it is done in the official repository.

rhys-vdw commented 2 years ago

What is the rationale for have a separate fork? I don't believe there are any downsides to supporting UPM in the main package. So long as edit code is under folders named Editor, Unity will just ignore the asmdefs and bundle everything into a single assembly (as was the way at the time).

bdovaz commented 2 years ago

What is the rationale for have a separate fork? I don't believe there are any downsides to supporting UPM in the main package. So long as edit code is under folders named Editor, Unity will just ignore the asmdefs and bundle everything into a single assembly (as was the way at the time).

Actually the repository is not assembled this way because everything is precompiled except the modules that are accompanied by an asmdef file. That is to say, in the editor folder there is no source code, only assemblies.

But regardless of what you say, if they can coexist the current version "asset mode" and the new version "package mode" there should be no problem and we can integrate everything in this repository and that's what I've been thinking since I started working on the PR.

bdovaz commented 2 years ago

@Demigiant I have already corrected the fact that it generates the assemblies with the correct target framework to maintain backward compatibility, I have added more commits to the PR and I have edited the PR post adding what I have modified.

Also you can test or give me access if you want to give you a hand to check the DOTween Pro part in case there is something that could have been affected by these changes.

I have reviewed the following projects:

DOTween DOTweenEditor DOTweenUpgradeManager DOTween_LooseScripts

Demigiant commented 2 years ago

Hey! I'm spending waaaaay more time in hospitals than I had estimated, so I'm a little overwhelmed right now. Since this is such a major change, I'll need at least two free days to check your merge request, @bdovaz, which I thought I would find max next week, but I might even need to go to the one after. I'm very conservative with my repos (until now they were more like open source that you can download but that I maintain by myself, which is my way to manage my mental health both because I'm very obsessive and because of how delicate it all is), so let's wait until I do that, but in the meantime I'm convincing myself to make this big change happen, and thank you for all the work :)

VasylRomanets-MoonActive commented 2 years ago

Get better soon, @Demigiant!

Demigiant commented 2 years ago

@VasylRomanets-MoonActive Thank you! I'm good, I'm just taking care of my aunt that is sick, but every day something new happens and the hospital(s) are far